From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: lucascouts Date: Thu, 22 Oct 2026 11:53:54 -0300 Subject: [PATCH] agent_ui: open a sub-thread from a selection in an agent message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Following up on one point of a long agent reply meant either asking in the same thread, where it derails the conversation, or opening a new thread by hand and pasting the excerpt, where the agent has lost every bit of context. This patch adds both kinds of sub-thread to the context menu of an agent message with text selected in it: story 039, 2026-10-22. Zed at the packaged commit has the protocol types for session/fork but no client call, so the patch also adds fork support to the agent connection, built so a later devtools sub-thread can reuse it. What it does. With text selected in an agent message, its context menu gains, after a separator, "Clean sub-thread" -- a new thread with the same agent in the same project, nothing inherited -- and "Sub-thread with context", which forks the agent session through ACP session/fork, so the new thread's agent knows the whole conversation so far. Both open the new thread focused with the excerpt in its composer as a Markdown quote followed by an empty line, and send nothing; the parent thread is left as it was. "Sub-thread with context" is offered only when the agent advertises session fork and also supports session load or resume (the panel could not open the forked session otherwise), and is disabled with "Wait for the turn to end" while the parent's turn runs. No entry appears without a selection, on a user message, a thinking block or a tool call, or in a subagent's thread view (operator decision 2026-10-22: a fork there would inherit the subagent's conversation while the link and the header name the root thread). The existing entries are kept. The new thread is linked to its parent in 0048's organization store (set_parent, fork kind clean or with_context), shows a "Sub-thread of: " header above its messages whose title opens the parent ("Sub-thread of a deleted thread", not clickable, once the parent is gone), and, for a fork, a band "Inherited N messages from " (N = the parent's user and agent messages at the fork; "Inherited the conversation of <title>" when N is unknown). Its threads-sidebar row carries a "↳" marker (IconName::ArrowDownRight, the closest existing glyph) through 0068's ThreadItem::badge, tooltip "Sub-thread of <parent title>", in every window, kept across a restart. A fork's link is recorded as soon as it opens; a clean sub-thread's on its first send, so a clean draft discarded or emptied before that records none. A fork keeps the parent's thread configuration -- the mode and config option values 0036 recorded for the parent session are applied to the fork, not the agent entry's defaults, and written under the fork's id so it reopens with them; with no record the fork gets the entry's defaults, as the parent did at its creation (operator decision 2026-10-21). Protocol status. session/fork is still unstable_ in ACP (agent-client-protocol 3.4.0, schema 1.12.0, feature unstable_session_fork, compiled through the workspace's existing "unstable" feature): a schema bump may rename the types or move the capability, so re-check on every Zed or adapter bump -- the entry disappears when an agent stops advertising sessionCapabilities.fork. Only the ACP v1 connection implements it. That reaches Claude Agent (Plus), whose unstable_forkSession resumes the SDK session with forkSession: true and replays no history: the adapter answers protocolVersion 1, so 0001's v2-first attempt falls back to v1. The v2 connection, the native agent and the test fakes keep the trait default, which refuses; an adapter that moves to v2 loses the entry until a v2 fork exists. How. crates/acp_thread/src/connection.rs: AgentConnection gains supports_fork_session and fork_session, defaulting to "not supported" exactly as session close does, and StubAgentConnection an opt-in with_fork_support builder. crates/acp_thread/src/acp_thread.rs: an AcpThread forked_from field, and is_draft_thread is false for a fork with no entries yet -- its session id is the agent's, fixed at the fork, so the metadata store keeps it and 0048's admit accepts the link. crates/agent_servers/src/acp/v1.rs: agent_supports_session_fork, SessionDirectories::into_fork_session_request (the parent thread's work directories and the project's MCP servers), and fork_session, which registers the forked session's AcpThread through register_answered_session -- new_session's post-response steps, moved into a helper both share -- and closes the fork when the caller is gone before the answer and the agent supports close. crates/agent_ui/src/sub_thread.rs (new, with sub_thread/forked_sub_thread.rs and sub_thread/message_menu.rs): AgentPanel::open_sub_thread(SubThreadRequest, focus), which takes a plain request rather than a ThreadView so a devtools sub-thread can call it; the clean path is 0045's new_thread_with_prefilled_prompt, the fork path opens the registered thread through create_agent_thread_with_server_for_external_session, so no session/load is sent and the view starts empty, and holds the forked AcpThread until the view's first RootThreadUpdated. conversation_view/sub_thread_header.rs (new) draws the header, the band and the never-used-fork state below. crates/sidebar/src/sub_thread_marker.rs (new) is the marker. Failure. A refused or failed fork -- an agent error, no working directories, no connection -- opens nothing, records no link, toasts "Couldn't create the sub-thread" and logs a warning with the parent session id and the agent's error code (code=none for a non-agent failure). A clean sub-thread whose agent is gone shows 0045's toast. A refused link keeps the thread open and logs both thread ids and the store's refusal. Reopening a fork that was never used, which the adapter answers -32002 (resource not found), keeps the row, the link and the header and shows "This sub-thread was never used and can't be reopened" instead of the generic load error. Every log line carries zeo_sub_thread event=<created|link_failed|open_failed|...> with ids, kinds and the excerpt's length only, never the excerpt or a title. Upstream touch points, hooks only. thread_view.rs (+20/-2): render_message_context_menu's offers_sub_threads flag, fed by 0045's offers_slash_commands at the agent-chunk call site and false at the thinking header, the menu_entries call, the header slot in render, and 0045's button closure passing focus. conversation_view.rs (+19/-4): the mod line, the sub_thread view state, the store observation, and the SubThreadUnavailable server state's arms. agent_panel.rs (+33/-7): 0045's new_thread_with_prefilled_prompt takes focus and returns the opened conversation; pub(crate) panel_workspace, show_agent_thread, has_open_project and create_agent_thread_with_server_for_external_session; one line in ensure_draft's reuse branch drops the pending link of an emptied clean draft. agent_servers/src/acp.rs: 0036's ConfigRecorder waits for the fork's inherited write before its first snapshot. agent_panel/tests (0045's): the new focus argument. agent_ui.rs and sidebar.rs: module lines, and in sidebar.rs one render_thread line between the project-name label and 0068's tag chips. No Cargo.toml, no Cargo.lock, no new crate or feature. Tests: 89 -- 87 in twelve new test files, acp_thread connection/fork_tests.rs (10); agent_servers acp/v1/fork_session_tests.rs (6) and fork_config_tests.rs (6); agent_ui sub_thread_tests/ pure (10), clean (11), fork (9), fork_abandoned (3), menu (8), menu_subagent (2), header (10), reopen (4), with support.rs; sidebar sidebar_sub_thread_tests.rs (8) -- and 2 in the test module of the new agent_ui conversation_view/sub_thread_header.rs. Group claude-agent-acp-plus, last of it: it calls 0045's new_thread_with_prefilled_prompt and extends 0036's config recorder, and the only agent that advertises fork today is the Plus adapter. It needs 0048's store and 0068's ThreadItem::badge, both ungrouped. 0052, 0057, 0058, 0063 and 0067 are regenerated on top with offsets, index lines and context only; 0063's devtools badge now sits before this patch's marker line. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --- diff --git a/crates/acp_thread/src/acp_thread.rs b/crates/acp_thread/src/acp_thread.rs index 5494dac..42a4f9d 100644 --- a/crates/acp_thread/src/acp_thread.rs +++ b/crates/acp_thread/src/acp_thread.rs @@ -4208,6 +4208,9 @@ pub struct AcpThread { subagents: IndexMap<acp_v2::SessionId, Entity<AcpThread>>, /// Set when this session is an ACP subagent announced by its parent. subagent_association: Option<subagents::SubagentAssociation>, + /// The session this one was forked from (`session/fork`), unlike `parent_session_id`, + /// which names the session that spawned a subagent. + forked_from: Option<acp_v2::SessionId>, provisional_title: Option<SharedString>, entries: Vec<AgentThreadEntry>, // Notices stay with the live session, but never enter conversation history or exports. @@ -4583,6 +4586,7 @@ impl AcpThread { let receipt_submissions = connection.receipt_submissions(&session_id, cx); Self { parent_session_id, + forked_from: None, session_info: AgentSessionInfo { session_id, work_dirs, @@ -4719,8 +4723,18 @@ impl AcpThread { cx.emit(AcpThreadEvent::AvailableCommandsUpdated(commands)); } + /// A fork's session id is fixed when it is forked, so an empty fork is not a draft: its + /// metadata keeps the id, and the organization store can link it to its parent (story 039). pub fn is_draft_thread(&self) -> bool { - self.entries().is_empty() + self.entries().is_empty() && self.forked_from.is_none() + } + + pub fn mark_forked_from(&mut self, parent: acp_v2::SessionId) { + self.forked_from = Some(parent); + } + + pub fn forked_from(&self) -> Option<&acp_v2::SessionId> { + self.forked_from.as_ref() } pub fn draft_prompt(&self) -> Option<&[acp_v2::ContentBlock]> { diff --git a/crates/acp_thread/src/connection.rs b/crates/acp_thread/src/connection.rs index 9db11be..0e938b2 100644 --- a/crates/acp_thread/src/connection.rs +++ b/crates/acp_thread/src/connection.rs @@ -164,6 +164,24 @@ pub trait AgentConnection { Task::ready(Err(anyhow::Error::msg("Closing sessions is not supported"))) } + /// Whether this agent can fork an existing session (`session/fork`, still `unstable_` + /// in ACP, so an agent must advertise it before a client may ask). + fn supports_fork_session(&self) -> bool { + false + } + + /// Fork `session_id` into a new session of the same conversation and return its thread. + /// The default refuses without sending anything (story 039, R6.4). + fn fork_session( + self: Rc<Self>, + _session_id: &acp_v2::SessionId, + _project: Entity<Project>, + _work_dirs: PathList, + _cx: &mut App, + ) -> Task<Result<Entity<AcpThread>>> { + Task::ready(Err(anyhow::Error::msg("Forking sessions is not supported"))) + } + /// Read the end of one of the session's tasks' output (`_claude/tasks/output`, /// version 2). Unsupported unless the connection overrides it. fn fetch_task_output( @@ -1092,6 +1110,7 @@ mod test_support { receipt_prompt: Arc<Mutex<Option<Vec<acp_v2::ContentBlock>>>>, next_truncate: Arc<Mutex<Option<oneshot::Receiver<()>>>>, close_session_calls: Arc<Mutex<Vec<acp_v2::SessionId>>>, + forked_from: Arc<Mutex<Vec<acp_v2::SessionId>>>, next_close_responses: Arc<Mutex<VecDeque<oneshot::Receiver<Result<()>>>>>, set_title_calls: Arc<Mutex<Vec<(acp_v2::SessionId, SharedString)>>>, cancel_calls: Arc<Mutex<Vec<acp_v2::SessionId>>>, @@ -1102,6 +1121,7 @@ mod test_support { supports_load_session: bool, supports_close_session: bool, announces_subagents: bool, + supports_fork_session: bool, close_sessions_on_release: bool, supports_session_additional_directories: bool, supports_set_title: bool, @@ -1141,6 +1161,7 @@ mod test_support { receipt_prompt: Default::default(), next_truncate: Default::default(), close_session_calls: Default::default(), + forked_from: Default::default(), next_close_responses: Default::default(), set_title_calls: Default::default(), cancel_calls: Default::default(), @@ -1153,6 +1174,7 @@ mod test_support { supports_load_session: false, supports_close_session: false, announces_subagents: false, + supports_fork_session: false, close_sessions_on_release: true, supports_session_additional_directories: false, supports_set_title: true, @@ -1312,6 +1334,17 @@ mod test_support { self } + /// Opt-in, so every existing stub user keeps the trait's refusing default. + pub fn with_fork_support(mut self) -> Self { + self.supports_fork_session = true; + self + } + + /// The parent session ids `fork_session` was asked to fork, in order. + pub fn forked_from(&self) -> Vec<acp_v2::SessionId> { + self.forked_from.lock().clone() + } + pub fn with_prompt_capabilities( mut self, prompt_capabilities_rx: watch::Receiver<acp_v2::PromptCapabilities>, @@ -1481,6 +1514,28 @@ mod test_support { } } + fn supports_fork_session(&self) -> bool { + self.supports_fork_session + } + + fn fork_session( + self: Rc<Self>, + session_id: &acp_v2::SessionId, + project: Entity<Project>, + work_dirs: PathList, + cx: &mut App, + ) -> Task<Result<Entity<AcpThread>>> { + if !self.supports_fork_session { + return Task::ready(Err(anyhow::Error::msg("Forking sessions is not supported"))); + } + self.forked_from.lock().push(session_id.clone()); + let forked_id = acp_v2::SessionId::new(StubSessionCounter::next(cx).to_string()); + let thread = self.create_session(forked_id, project, work_dirs, None, cx); + let parent = session_id.clone(); + thread.update(cx, |thread, _| thread.mark_forked_from(parent)); + Task::ready(Ok(thread)) + } + fn supports_session_additional_directories(&self) -> bool { self.supports_session_additional_directories } @@ -1500,6 +1555,17 @@ mod test_support { if let Some(error) = self.next_load_error.lock().take() { return Task::ready(Err(error)); } + // A fork is opened by id the way ACP opens it: the thread registered at fork + // time, not a second one under the same id (story 039, task 4.3). + let registered_fork = self + .sessions + .lock() + .get(&session_id) + .and_then(|session| session.thread.upgrade()) + .filter(|thread| thread.read(cx).forked_from().is_some()); + if let Some(thread) = registered_fork { + return Task::ready(Ok(thread)); + } let thread = self.create_session(session_id, project, work_dirs, title, cx); Task::ready(Ok(thread)) } @@ -1804,6 +1870,9 @@ mod test_support { #[cfg(any(test, feature = "test-support"))] pub use test_support::*; +#[cfg(test)] +mod fork_tests; + #[cfg(test)] mod tests { use super::*; diff --git a/crates/acp_thread/src/connection/fork_tests.rs b/crates/acp_thread/src/connection/fork_tests.rs new file mode 100644 index 0000000..8dc96b3 --- /dev/null +++ b/crates/acp_thread/src/connection/fork_tests.rs @@ -0,0 +1,460 @@ +// Story 039, tasks 3.1 and 3.3 — fork as an optional capability of every agent connection +// (R6.4, R3.3; Q4, Q5): `AgentConnection::supports_fork_session` / `fork_session` with refusing +// defaults, the test stub's opt-in `StubAgentConnection::with_fork_support` / `forked_from`, and +// the `AcpThread` fork mark that keeps an empty fork from being a draft. +// +// Materialize with, in `crates/acp_thread/src/connection.rs` (a non-mod-rs file, so this resolves +// to `crates/acp_thread/src/connection/fork_tests.rs`): +// #[cfg(test)] +// mod fork_tests; +// Filter: `cargo test -p acp_thread fork` (paths `connection::fork_tests::…`). +// +// Contract (design.md *Fork in the connection*): +// fn supports_fork_session(&self) -> bool { false } +// fn fork_session(self: Rc<Self>, session_id: &acp_v2::SessionId, project: Entity<Project>, +// work_dirs: PathList, cx: &mut App) -> Task<Result<Entity<AcpThread>>> +// default: an error, nothing sent ("Forking sessions is not supported") +// StubAgentConnection::with_fork_support(self) -> Self (its forked thread is marked forked) +// StubAgentConnection::forked_from(&self) -> Vec<acp_v2::SessionId> +// +// Task 3.3 shares this file (design.md *A forked thread is not a draft*): +// AcpThread::mark_forked_from(&mut self, parent: acp_v2::SessionId) +// AcpThread::forked_from(&self) -> Option<&acp_v2::SessionId> +// is_draft_thread() == entries().is_empty() && forked_from().is_none() +// +// Hostile halves first (identity: a fork is a NEW session of the SAME conversation): +// - wrong collapse: the fork must never hand back the parent's session or thread, and two forks +// of one parent are two sessions, not one; +// - wrong split: the forked session, asked for again by id, is the thread the fork registered — +// not a second thread under the same id. THIS ONE GOES BEYOND design.md's stub contract: it is +// what lets task 4.3 open the fork over the stub with "no load request sent" (design.md's +// Decision relies on `AcpConnection::open_or_create_session` returning the registered thread; +// the stub's `load_session` built a fresh thread at the series tip). + +use std::cell::Cell; +use std::path::Path; +use std::rc::Rc; + +use agent_client_protocol::{v1::schema as acp_v1, v2::schema as acp_v2}; +use gpui::{App, Entity, SharedString, Task, TestAppContext}; +use project::{AgentId, FakeFs, Project}; +use settings::SettingsStore; +use util::path_list::PathList; + +use super::*; +use crate::{AcpThread, AgentThreadEntry}; + +fn init(cx: &mut TestAppContext) { + cx.update(|cx| { + let mut settings_store = SettingsStore::test(cx); + settings_store.register_setting::<feature_flags::FeatureFlagsSettings>(); + cx.set_global(settings_store); + }); +} + +async fn project(cx: &mut TestAppContext) -> Entity<Project> { + let fs = FakeFs::new(cx.executor()); + Project::test(fs, [], cx).await +} + +fn work_dirs() -> PathList { + PathList::new(&[Path::new("/work/app"), Path::new("/work/lib")]) +} + +fn agent_says(text: &str) -> acp_v1::SessionUpdate { + acp_v1::SessionUpdate::AgentMessageChunk(acp_v1::ContentChunk::new(text.into())) +} + +fn session_of(thread: &Entity<AcpThread>, cx: &TestAppContext) -> acp_v2::SessionId { + thread.read_with(cx, |thread, _| thread.session_id().clone()) +} + +fn entry_count(thread: &Entity<AcpThread>, cx: &TestAppContext) -> usize { + thread.read_with(cx, |thread, _| thread.entries().len()) +} + +fn user_messages(thread: &Entity<AcpThread>, cx: &TestAppContext) -> usize { + thread.read_with(cx, |thread, _| { + thread + .entries() + .iter() + .filter(|entry| matches!(entry, AgentThreadEntry::UserMessage(_))) + .count() + }) +} + +/// A session of `connection` that has had one turn: a user message and the agent's reply. +async fn parent_with_a_turn( + connection: &Rc<StubAgentConnection>, + project: &Entity<Project>, + cx: &mut TestAppContext, +) -> Entity<AcpThread> { + let thread = cx + .update(|cx| { + connection + .clone() + .new_session(project.clone(), work_dirs(), cx) + }) + .await + .expect("the stub opens a session"); + connection.set_next_prompt_updates(vec![agent_says("Noted.")]); + thread + .update(cx, |thread, cx| thread.send_raw("Remember QZ-1.", cx)) + .await + .expect("the parent's turn completes"); + cx.run_until_parked(); + assert_eq!( + entry_count(&thread, cx), + 2, + "fixture: the parent has a turn" + ); + thread +} + +async fn fork( + connection: &Rc<StubAgentConnection>, + parent: &acp_v2::SessionId, + project: &Entity<Project>, + cx: &mut TestAppContext, +) -> anyhow::Result<Entity<AcpThread>> { + let task = cx.update(|cx| { + connection + .clone() + .fork_session(parent, project.clone(), work_dirs(), cx) + }); + let result = task.await; + cx.run_until_parked(); + result +} + +/// An agent connection that implements only what the trait requires, so its fork answers are the +/// trait's defaults. It counts the sessions it is asked to create: a default fork creates none. +#[derive(Default)] +struct ForklessAgentConnection { + new_sessions: Cell<usize>, +} + +impl AgentConnection for ForklessAgentConnection { + fn agent_id(&self) -> AgentId { + AgentId::new("forkless") + } + + fn telemetry_id(&self) -> SharedString { + "forkless".into() + } + + fn new_session( + self: Rc<Self>, + _project: Entity<Project>, + _work_dirs: PathList, + _cx: &mut App, + ) -> Task<anyhow::Result<Entity<AcpThread>>> { + self.new_sessions.set(self.new_sessions.get() + 1); + Task::ready(Err(anyhow::anyhow!("no session is created by this fake"))) + } + + fn auth_methods(&self) -> &[acp_v2::AuthMethod] { + &[] + } + + fn authenticate( + &self, + _method: acp_v2::AuthMethodId, + _cx: &mut App, + ) -> Task<anyhow::Result<()>> { + Task::ready(Ok(())) + } + + fn cancel(&self, _session_id: &acp_v2::SessionId, _cx: &mut App) -> anyhow::Result<()> { + Ok(()) + } + + fn into_any(self: Rc<Self>) -> Rc<dyn std::any::Any> { + self + } +} + +// R6.4 — a connection type that does not implement fork reports it and refuses, creating nothing. +#[gpui::test] +async fn fork_default_reports_unsupported_and_refuses_without_side_effects( + cx: &mut TestAppContext, +) { + init(cx); + let project = project(cx).await; + let connection = Rc::new(ForklessAgentConnection::default()); + assert!( + !connection.supports_fork_session(), + "the default is no fork" + ); + + let parent = acp_v2::SessionId::new("parent-of-a-forkless-agent"); + let result = cx + .update(|cx| { + connection + .clone() + .fork_session(&parent, project.clone(), work_dirs(), cx) + }) + .await; + cx.run_until_parked(); + + let error = result.expect_err("R6.4: the default answers a fork request with an error"); + assert!( + format!("{error:#}") + .to_lowercase() + .contains("not supported"), + "the error says fork is not supported, got: {error:#}" + ); + assert_eq!( + connection.new_sessions.get(), + 0, + "a refused fork must not fall back to creating a session" + ); +} + +// R6.4 over the stub every existing test uses: without the opt-in it keeps the default — no fork +// reported, a refusal, nothing recorded, the parent untouched. +#[gpui::test] +async fn fork_stub_without_fork_support_refuses_and_records_nothing(cx: &mut TestAppContext) { + init(cx); + let project = project(cx).await; + let connection = Rc::new(StubAgentConnection::new()); + let parent = parent_with_a_turn(&connection, &project, cx).await; + let parent_id = session_of(&parent, cx); + assert!(!connection.supports_fork_session()); + + let result = fork(&connection, &parent_id, &project, cx).await; + + assert!(result.is_err(), "a stub without fork refuses"); + assert!(connection.forked_from().is_empty(), "nothing recorded"); + assert_eq!(entry_count(&parent, cx), 2, "the parent is untouched"); +} + +// Hostile half (wrong collapse) — the fork is a new session and a new thread, never the parent's, +// and it starts empty although the parent has a turn: the stub replays nothing. +#[gpui::test] +async fn fork_hostile_never_hands_back_the_parent_session(cx: &mut TestAppContext) { + init(cx); + let project = project(cx).await; + let connection = Rc::new(StubAgentConnection::new().with_fork_support()); + assert!( + connection.supports_fork_session(), + "the opt-in reports fork" + ); + let parent = parent_with_a_turn(&connection, &project, cx).await; + let parent_id = session_of(&parent, cx); + + let forked = fork(&connection, &parent_id, &project, cx) + .await + .expect("a stub with fork forks"); + + assert_ne!(session_of(&forked, cx), parent_id, "a NEW session id"); + assert_ne!(forked, parent, "a NEW thread, not the parent's entity"); + assert_eq!( + entry_count(&forked, cx), + 0, + "nothing replayed into the fork" + ); + assert_eq!( + forked.read_with(cx, |thread, _| thread.forked_from().cloned()), + Some(parent_id.clone()), + "the forked thread is marked with its parent" + ); + assert_eq!( + parent.read_with(cx, |thread, _| thread.forked_from().cloned()), + None, + "the mark is the fork's, never the parent's" + ); + assert_eq!(entry_count(&parent, cx), 2, "the parent is untouched"); + assert_eq!( + forked.read_with(cx, |thread, _| thread.work_dirs().cloned()), + Some(work_dirs()), + "the fork runs in the directories it was given" + ); +} + +// Hostile half (wrong collapse, again) — two forks of one parent are two sessions and two threads. +#[gpui::test] +async fn fork_hostile_two_forks_of_one_parent_are_two_sessions(cx: &mut TestAppContext) { + init(cx); + let project = project(cx).await; + let connection = Rc::new(StubAgentConnection::new().with_fork_support()); + let parent = parent_with_a_turn(&connection, &project, cx).await; + let parent_id = session_of(&parent, cx); + + let first = fork(&connection, &parent_id, &project, cx) + .await + .expect("first fork"); + let second = fork(&connection, &parent_id, &project, cx) + .await + .expect("second fork"); + + assert_ne!(first, second); + assert_ne!(session_of(&first, cx), session_of(&second, cx)); + assert_eq!(connection.forked_from(), vec![parent_id.clone(), parent_id]); +} + +// Hostile half (wrong split, the converse) — the forked session asked for again by its id is the +// thread the fork registered, still empty: no second thread, no replay. Task 4.3 relies on it to +// open the fork in a view over the stub the way Zeo opens it over ACP (design.md, Decision). +#[gpui::test] +async fn fork_hostile_a_forked_session_reopened_by_id_is_the_registered_thread( + cx: &mut TestAppContext, +) { + init(cx); + let project = project(cx).await; + let connection = Rc::new( + StubAgentConnection::new() + .with_supports_load_session(true) + .with_fork_support(), + ); + let parent = parent_with_a_turn(&connection, &project, cx).await; + let parent_id = session_of(&parent, cx); + let forked = fork(&connection, &parent_id, &project, cx) + .await + .expect("fork"); + let forked_id = session_of(&forked, cx); + + let reopened = cx + .update(|cx| { + connection.clone().load_session( + forked_id.clone(), + project.clone(), + work_dirs(), + None, + cx, + ) + }) + .await + .expect("the forked session can be opened by id"); + cx.run_until_parked(); + + assert_eq!(reopened, forked, "the same thread, not a second one"); + assert_eq!(entry_count(&reopened, cx), 0); +} + +// The stub records the parent it was asked to fork — not the new id, and the right one of two. +#[gpui::test] +async fn fork_stub_records_the_parent_it_was_asked_to_fork(cx: &mut TestAppContext) { + init(cx); + let project = project(cx).await; + let connection = Rc::new(StubAgentConnection::new().with_fork_support()); + let _other = parent_with_a_turn(&connection, &project, cx).await; + let parent = parent_with_a_turn(&connection, &project, cx).await; + let parent_id = session_of(&parent, cx); + + let forked = fork(&connection, &parent_id, &project, cx) + .await + .expect("fork"); + + assert_eq!(connection.forked_from(), vec![parent_id]); + assert!( + !connection.forked_from().contains(&session_of(&forked, cx)), + "the new id is not what is recorded" + ); +} + +// R3.4 at the connection: a prompt on the fork lands in the fork and never in the parent. +#[gpui::test] +async fn fork_a_prompt_on_the_fork_reaches_the_fork_only(cx: &mut TestAppContext) { + init(cx); + let project = project(cx).await; + let connection = Rc::new(StubAgentConnection::new().with_fork_support()); + let parent = parent_with_a_turn(&connection, &project, cx).await; + let parent_id = session_of(&parent, cx); + let forked = fork(&connection, &parent_id, &project, cx) + .await + .expect("fork"); + + connection.set_next_prompt_updates(vec![agent_says("QZ-1")]); + forked + .update(cx, |thread, cx| thread.send_raw("Which token?", cx)) + .await + .expect("the fork answers"); + cx.run_until_parked(); + + assert_eq!(user_messages(&forked, cx), 1, "the fork got the prompt"); + assert_eq!(user_messages(&parent, cx), 1, "the parent kept its own one"); + assert_eq!(entry_count(&parent, cx), 2, "the parent is untouched"); +} + +// ── Task 3.3: a forked thread is not a draft (R3.3) ───────────────────────── + +fn is_draft(thread: &Entity<AcpThread>, cx: &TestAppContext) -> bool { + thread.read_with(cx, |thread, _| thread.is_draft_thread()) +} + +fn mark(thread: &Entity<AcpThread>, parent: &str, cx: &mut TestAppContext) { + let parent = acp_v2::SessionId::new(parent); + thread.update(cx, |thread, _| thread.mark_forked_from(parent)); +} + +async fn empty_thread( + connection: &Rc<StubAgentConnection>, + cx: &mut TestAppContext, +) -> Entity<AcpThread> { + let project = project(cx).await; + cx.update(|cx| connection.clone().new_session(project, work_dirs(), cx)) + .await + .expect("a session") +} + +// Hostile half first — marking is what turns an empty thread from a draft into a real one; an +// empty unmarked thread stays a draft exactly as upstream has it. +#[gpui::test] +async fn fork_draft_an_empty_thread_is_a_draft_until_marked_forked(cx: &mut TestAppContext) { + init(cx); + let connection = Rc::new(StubAgentConnection::new()); + let thread = empty_thread(&connection, cx).await; + assert_eq!(entry_count(&thread, cx), 0, "fixture: empty"); + assert!( + is_draft(&thread, cx), + "an empty unforked thread is a draft (unchanged)" + ); + assert_eq!(thread.read_with(cx, |t, _| t.forked_from().cloned()), None); + + mark(&thread, "parent-1", cx); + + assert!( + !is_draft(&thread, cx), + "an empty FORKED thread is not a draft" + ); + assert_eq!( + thread.read_with(cx, |t, _| t.forked_from().cloned()), + Some(acp_v2::SessionId::new("parent-1")) + ); +} + +// The mark outlives the thread's first entries. +#[gpui::test] +async fn fork_draft_the_mark_survives_entries(cx: &mut TestAppContext) { + init(cx); + let connection = Rc::new(StubAgentConnection::new()); + let thread = empty_thread(&connection, cx).await; + mark(&thread, "parent-1", cx); + + connection.set_next_prompt_updates(vec![agent_says("Here.")]); + thread + .update(cx, |thread, cx| thread.send_raw("Still there?", cx)) + .await + .expect("a turn"); + cx.run_until_parked(); + + assert_eq!(entry_count(&thread, cx), 2, "fixture: entries arrived"); + assert_eq!( + thread.read_with(cx, |t, _| t.forked_from().cloned()), + Some(acp_v2::SessionId::new("parent-1")) + ); + assert!(!is_draft(&thread, cx)); +} + +// Unchanged upstream behaviour: an unmarked thread with entries is not a draft. +#[gpui::test] +async fn fork_draft_an_unmarked_thread_with_entries_is_not_a_draft(cx: &mut TestAppContext) { + init(cx); + let project = project(cx).await; + let connection = Rc::new(StubAgentConnection::new()); + let thread = parent_with_a_turn(&connection, &project, cx).await; + + assert!(!is_draft(&thread, cx)); + assert_eq!(thread.read_with(cx, |t, _| t.forked_from().cloned()), None); +} diff --git a/crates/agent_servers/src/acp.rs b/crates/agent_servers/src/acp.rs index ab9ed09..9a9d16d 100644 --- a/crates/agent_servers/src/acp.rs +++ b/crates/agent_servers/src/acp.rs @@ -519,6 +519,10 @@ impl ConfigRecorder { config_options: Option<&ConfigOptions>, modes: Option<Rc<RefCell<acp::SessionModeState>>>, mode_rx: watch::Receiver<()>, + // A write already under way for this session (a fork's inherited + // record): the first snapshot waits for it, so that write never lands + // over a newer snapshot. + pending_write: Option<futures::channel::oneshot::Receiver<()>>, cx: &App, ) -> Self { // Taken from the sender, the receiver starts at the current version: @@ -527,7 +531,7 @@ impl ConfigRecorder { let options = config_options.map(|options| options.config_options.clone()); let mut mode_rx = Some(mode_rx); let task = cx.spawn(async move |cx| { - let mut previous_write: Option<futures::channel::oneshot::Receiver<()>> = None; + let mut previous_write: Option<futures::channel::oneshot::Receiver<()>> = pending_write; while either_changed(&mut options_rx, &mut mode_rx).await { // The watch coalesces wakes, so the cells are read as they are // now rather than per signal. diff --git a/crates/agent_servers/src/acp/v1.rs b/crates/agent_servers/src/acp/v1.rs index c9e6a7e..eddb66c 100644 --- a/crates/agent_servers/src/acp/v1.rs +++ b/crates/agent_servers/src/acp/v1.rs @@ -905,11 +905,171 @@ impl AcpConnection { ); } + /// What a session the agent has just created needs before anyone can use it: its + /// configuration applied, its thread built and registered. Shared by `new_session` + /// and `fork_session`, which differ only in the request and in `inherited`. + /// + /// Without `inherited`, the entry's default mode and config options apply. With it + /// (a fork whose parent has a record), the session opens as a reopen opens one: + /// only the recorded values the agent offers are sent, never a default, and the + /// record is written under the new session's id so it reopens with it (R6.5). + fn register_answered_session( + self: &Rc<Self>, + session_id: acp::SessionId, + modes: Option<acp::SessionModeState>, + config_options: Option<Vec<acp::SessionConfigOption>>, + inherited: Option<InheritedConfig>, + project: Entity<Project>, + work_dirs: PathList, + cx: &mut AsyncApp, + ) -> Result<Entity<AcpThread>> { + let (modes, config_options) = config_state(modes, config_options)?; + // Created before the session is registered, so the mode default's + // request can signal it once the agent answers. + let mode_trigger = ModeTrigger::new(); + let recorded = inherited.as_ref().map(|inherited| &inherited.record); + + // The entry's mode default is new-session only, as on reopen: over a + // record it would replace the mode the parent's thread chose. + let default_mode = self.defaults.mode().filter(|_| recorded.is_none()); + if let Some(default_mode) = default_mode { + let default_mode = acp::SessionModeId::new(default_mode.0); + if let Some(modes) = modes.as_ref() { + let mut modes_ref = modes.borrow_mut(); + let has_mode = modes_ref + .available_modes + .iter() + .any(|mode| mode.id == default_mode); + + if has_mode { + let initial_mode_id = modes_ref.current_mode_id.clone(); + + cx.spawn({ + let default_mode = default_mode.clone(); + let session_id = session_id.clone(); + let modes = modes.clone(); + let mode_trigger = mode_trigger.clone(); + let conn = self.connection.clone(); + async move |_| { + let result = conn + .send_request(acp::SetSessionModeRequest::new( + session_id, + default_mode, + )) + .block_task() + .await + .log_err(); + + if result.is_none() { + modes.borrow_mut().current_mode_id = initial_mode_id; + } + // After the answer, and after the revert on + // error, so the record holds the agent's mode. + mode_trigger.fire(); + } + }) + .detach(); + + modes_ref.current_mode_id = default_mode; + } else { + let available_modes = modes_ref + .available_modes + .iter() + .map(|mode| format!("- `{}`: {}", mode.id, mode.name)) + .collect::<Vec<_>>() + .join("\n"); + + log::warn!( + "`{default_mode}` is not valid {} mode. Available options:\n{available_modes}", + self.id.0, + ); + } + } + } + + let session_id = acp_v2::SessionId::new(session_id.0); + let default_config_task = config_options + .as_ref() + .map(|options| self.apply_session_config_options(&session_id, options, recorded, cx)); + + let action_log = cx.new(|_| ActionLog::new(project.clone())); + let thread: Entity<AcpThread> = cx.new(|cx| { + AcpThread::new( + None, + None, + Some(work_dirs), + self.clone(), + project, + action_log, + session_id.clone(), + // ACP doesn't currently support per-session prompt capabilities or changing capabilities dynamically. + watch::Receiver::constant(self.prompt_capabilities()), + cx, + ) + }); + + let recorded_mode = recorded.and_then(|recorded| recorded.mode.as_deref()); + if let (Some(modes), Some(recorded_mode)) = (modes.as_ref(), recorded_mode) { + self.apply_recorded_mode(&session_id, modes, recorded_mode, &mode_trigger, cx); + } + + cx.update(|cx| { + self.register_session( + session_id.clone(), + &thread, + modes, + config_options, + default_config_task, + mode_trigger, + cx, + ); + let inherited_write = + inherited.map(|inherited| self.record_inherited_config(&session_id, inherited, cx)); + self.start_config_recorder(&session_id, inherited_write, cx); + }); + Ok(thread) + } + + /// Writes a fork's inherited record under the fork's own session id, so the fork + /// reopens with its parent's configuration even when nothing changes and the + /// change-driven recorder never writes. A failure is logged with both session ids + /// and leaves the fork open. The returned receiver resolves once the write ends. + fn record_inherited_config( + &self, + session_id: &acp_v2::SessionId, + inherited: InheritedConfig, + cx: &App, + ) -> futures::channel::oneshot::Receiver<()> { + let (written_tx, written_rx) = futures::channel::oneshot::channel(); + let write = thread_config_store::write(&self.id, session_id, &inherited.record, cx); + let agent_id = self.id.clone(); + let fork = session_id.clone(); + cx.background_spawn(async move { + if let Err(error) = write.await { + log::warn!( + "failed to record the inherited thread config of {} fork {} of session {}: \ + {error:#}", + agent_id, + fork.0, + inherited.parent.0 + ); + } + written_tx.send(()).ok(); + }) + .detach(); + written_rx + } + /// Starts recording a registered session's config. Call it only once the /// session's config cells are attached and their restoration (or the /// entry defaults) has been dispatched, so nothing an agent replays before /// the record was read can be written over it. - fn start_config_recorder(&self, session_id: &acp_v2::SessionId, cx: &App) { + fn start_config_recorder( + &self, + session_id: &acp_v2::SessionId, + pending_write: Option<futures::channel::oneshot::Receiver<()>>, + cx: &App, + ) { let mut sessions = self.sessions.borrow_mut(); let Some(session) = sessions.get_mut(session_id) else { return; @@ -921,6 +1081,7 @@ impl AcpConnection { session.config_options.as_ref(), session.session_modes.clone(), mode_rx, + pending_write, cx, )); } @@ -999,6 +1160,12 @@ impl AcpConnection { self.agent_capabilities.session_capabilities.close.is_some() } + /// `session/fork` is still `unstable_` in ACP: an agent must advertise it before a + /// client may send it. + fn agent_supports_session_fork(&self) -> bool { + self.agent_capabilities.session_capabilities.fork.is_some() + } + /// Keyed on the capability alone: the entry id is whatever the user named it, and /// the agent's self-reported name differs between the fork and the published package. /// @@ -1228,7 +1395,9 @@ impl AcpConnection { thread.invalidate_replayed_subagent_state(cx) }); } - cx.update(|cx| connection_state.start_config_recorder(&session_id, cx)); + cx.update(|cx| { + connection_state.start_config_recorder(&session_id, None, cx) + }); connection_state.remove_pending_session(&session_id, thread.entity_id()); return Ok(()); } @@ -1267,10 +1436,11 @@ impl AcpConnection { } /// Applies the config options of a session as it opens. A newly created - /// session, a fork included, passes no record and gets the entry's - /// defaults. A reopened one passes its record and gets only the recorded - /// values the agent offers: the defaults never apply on reopen, so an - /// option without a usable record keeps the value the agent reports. + /// session, or a fork whose parent has no record, passes no record and gets + /// the entry's defaults. A reopened one, or a fork whose parent has a record, + /// passes that record and gets only the recorded values the agent offers: + /// the defaults never apply then, so an option without a usable record keeps + /// the value the agent reports. fn apply_session_config_options( &self, session_id: &acp_v2::SessionId, @@ -1439,6 +1609,12 @@ impl AcpConnection { } } +/// The record a fork inherits from its parent session, and that parent's id for the log. +struct InheritedConfig { + parent: acp_v2::SessionId, + record: RecordedConfig, +} + /// What restoring one config option asks of the agent. #[derive(Debug, Clone, PartialEq)] enum Choice { @@ -1582,6 +1758,12 @@ mod recorder_mode_tests; #[cfg(test)] mod round_trip_tests; +#[cfg(test)] +mod fork_session_tests; + +#[cfg(test)] +mod fork_config_tests; + #[derive(Clone, Debug, PartialEq, Eq)] struct SessionDirectories { cwd: PathBuf, @@ -1614,6 +1796,17 @@ impl SessionDirectories { .additional_directories(self.additional_directories) .mcp_servers(mcp_servers) } + + /// `session/fork` is `unstable_` in ACP (feature `unstable_session_fork`). + fn into_fork_session_request( + self, + session_id: acp_v2::SessionId, + mcp_servers: Vec<acp::McpServer>, + ) -> acp::ForkSessionRequest { + acp::ForkSessionRequest::new(acp::SessionId::new(session_id.0), self.cwd) + .additional_directories(self.additional_directories) + .mcp_servers(mcp_servers) + } } fn session_directories_from_work_dirs( @@ -1707,7 +1900,6 @@ impl AgentConnection for AcpConnection { Ok(directories) => directories, Err(error) => return Task::ready(Err(error)), }; - let name = self.id.0.clone(); let mcp_servers = mcp_servers_for_project(&project, cx); cx.spawn(async move |cx| { @@ -1767,99 +1959,15 @@ impl AgentConnection for AcpConnection { .context("ACP new-session response callback was cancelled")? .map_err(map_acp_error)?; - let (modes, config_options) = config_state(response.modes, response.config_options)?; - // Created before the session is registered, so the mode default's - // request can signal it once the agent answers. - let mode_trigger = ModeTrigger::new(); - - let default_mode = self.defaults.mode(); - if let Some(default_mode) = default_mode { - let default_mode = acp::SessionModeId::new(default_mode.0); - if let Some(modes) = modes.as_ref() { - let mut modes_ref = modes.borrow_mut(); - let has_mode = modes_ref - .available_modes - .iter() - .any(|mode| mode.id == default_mode); - - if has_mode { - let initial_mode_id = modes_ref.current_mode_id.clone(); - - cx.spawn({ - let default_mode = default_mode.clone(); - let session_id = response.session_id.clone(); - let modes = modes.clone(); - let mode_trigger = mode_trigger.clone(); - let conn = self.connection.clone(); - async move |_| { - let result = conn - .send_request(acp::SetSessionModeRequest::new( - session_id, - default_mode, - )) - .block_task() - .await - .log_err(); - - if result.is_none() { - modes.borrow_mut().current_mode_id = initial_mode_id; - } - // After the answer, and after the revert on - // error, so the record holds the agent's mode. - mode_trigger.fire(); - } - }) - .detach(); - - modes_ref.current_mode_id = default_mode; - } else { - let available_modes = modes_ref - .available_modes - .iter() - .map(|mode| format!("- `{}`: {}", mode.id, mode.name)) - .collect::<Vec<_>>() - .join("\n"); - - log::warn!( - "`{default_mode}` is not valid {name} mode. Available options:\n{available_modes}", - ); - } - } - } - - let session_id = acp_v2::SessionId::new(response.session_id.0); - let default_config_task = config_options.as_ref().map(|options| { - self.apply_session_config_options(&session_id, options, None, cx) - }); - - let action_log = cx.new(|_| ActionLog::new(project.clone())); - let thread: Entity<AcpThread> = cx.new(|cx| { - AcpThread::new( - None, - None, - Some(work_dirs), - self.clone(), - project, - action_log, - session_id.clone(), - // ACP doesn't currently support per-session prompt capabilities or changing capabilities dynamically. - watch::Receiver::constant(self.prompt_capabilities()), - cx, - ) - }); - - cx.update(|cx| { - self.register_session( - session_id.clone(), - &thread, - modes, - config_options, - default_config_task, - mode_trigger, - cx, - ); - self.start_config_recorder(&session_id, cx); - }); + let thread = self.register_answered_session( + response.session_id, + response.modes, + response.config_options, + None, + project, + work_dirs, + cx, + )?; registered .send(()) @@ -1883,6 +1991,116 @@ impl AgentConnection for AcpConnection { .is_some() } + fn supports_fork_session(&self) -> bool { + self.agent_supports_session_fork() + } + + /// Sends `session/fork` (still `unstable_` in ACP) for `session_id` with the parent + /// thread's directories and the project's MCP servers, and registers the thread of the + /// session the agent answers with, marked as forked from the parent. + fn fork_session( + self: Rc<Self>, + session_id: &acp_v2::SessionId, + project: Entity<Project>, + work_dirs: PathList, + cx: &mut App, + ) -> Task<Result<Entity<AcpThread>>> { + if !self.agent_supports_session_fork() { + return Task::ready(Err(anyhow!(LoadError::Other( + "Forking sessions is not supported by this agent.".into() + )))); + } + let directories = match self.session_directories_from_work_dirs(&work_dirs) { + Ok(directories) => directories, + Err(error) => return Task::ready(Err(error)), + }; + let parent = session_id.clone(); + let mcp_servers = mcp_servers_for_project(&project, cx); + // Read before the request, as a reopen reads before `session/load`. Keyed by + // this entry and the parent, so another entry's record of the same id is not it. + let parent_record = thread_config_store::read(&self.id, &parent, cx); + + cx.spawn(async move |cx| { + let (response_sender, response_received) = futures::channel::oneshot::channel(); + let (registered, registration_finished) = futures::channel::oneshot::channel(); + let request = self.connection.prepare_request( + directories.into_fork_session_request(parent.clone(), mcp_servers), + ); + let cancellation = request.cancellation_handle(); + request + .on_receiving_result({ + let connection = self.connection.clone(); + let supports_close = self.agent_supports_session_close(); + // The same abandoned-session close as `new_session`: a fork nobody + // registered would otherwise stay open in the agent. + async move |result: Result< + acp::ForkSessionResponse, + agent_client_protocol::Error, + >| { + let session_id = result + .as_ref() + .ok() + .map(|response| response.session_id.clone()); + let registered = response_sender.send(result).is_ok() + && registration_finished.await.is_ok(); + if !registered + && supports_close + && let Some(session_id) = session_id + { + connection + .prepare_request(acp::CloseSessionRequest::new(session_id)) + .on_receiving_result( + async move |result: Result< + acp::CloseSessionResponse, + agent_client_protocol::Error, + >| { + if let Err(error) = result { + log::warn!( + "ACP abandoned-fork close failed with code {}", + error.code + ); + } + Ok(()) + }, + ) + .log_err(); + } + Ok(()) + } + }) + .map_err(map_acp_error)?; + // Armed after the publishing call returns, as in `new_session`: early SDK + // cancellation is not latched. + let _request_guard = util::defer(move || { + cancellation.cancel().log_err(); + }); + let response = response_received + .await + .context("ACP fork-session response callback was cancelled")? + .map_err(map_acp_error)?; + + let inherited = parent_record.map(|record| InheritedConfig { + parent: parent.clone(), + record, + }); + let thread = self.register_answered_session( + response.session_id, + response.modes, + response.config_options, + inherited, + project, + work_dirs, + cx, + )?; + thread.update(cx, |thread, _| thread.mark_forked_from(parent)); + + registered.send(()).map_err(|_| { + anyhow!("ACP fork-session ordering callback ended before registration") + })?; + Ok(thread) + }) + } + fn supports_session_additional_directories(&self) -> bool { self.agent_capabilities .session_capabilities diff --git a/crates/agent_servers/src/acp/v1/fork_config_tests.rs b/crates/agent_servers/src/acp/v1/fork_config_tests.rs new file mode 100644 index 0000000..c00336c --- /dev/null +++ b/crates/agent_servers/src/acp/v1/fork_config_tests.rs @@ -0,0 +1,656 @@ +//! Story 039, task 10.1 — a fork keeps the parent's thread configuration (R6.5, R6.6; Q4, Q5): +//! the real `AcpConnection` against a scripted agent on the SDK's in-memory channel, with the +//! production client handler set, so the fork's `session/set_config_option` and +//! `session/set_mode` requests travel exactly as they do to the Plus adapter. +//! +//! Materialize with, in `crates/agent_servers/src/acp/v1.rs`, beside `mod fork_session_tests;`: +//! #[cfg(test)] +//! mod fork_config_tests; +//! Filter: `cargo test -p agent_servers fork_config` (paths `acp::v1::fork_config_tests::…`). +//! +//! Contract (design.md *Fork over ACP v1* → *Configuration of the fork*, decided 2026-10-21 after +//! the 3.2 tech review, finding F1): +//! - with a record for (entry, parent): the fork opens as a reopen opens a session — only the +//! recorded values the agent offers are sent, and the entry defaults never apply; +//! - without one: the new-session path, the entry defaults (R6.6); +//! - afterwards the fork's own session id holds the parent's record, so it reopens with it. +//! +//! Hostile halves first: the defaults must not win over a record (wrong collapse of "fork" into +//! "new session"); the record of the same session under ANOTHER entry must not be taken for the +//! parent's (a third element sharing the session id); the fork's record must exist even when the +//! agent already reports the parent's values and nothing changes — the case a change-driven +//! recorder never writes. +//! +//! Two agent shapes, because Zeo keeps `SessionModeState` only when an agent has no config +//! options (`config_state`): the Plus shape carries the mode as the `mode` select option; the +//! legacy shape carries it as a session mode, restored with `session/set_mode`. + +use std::collections::HashMap as StdHashMap; +use std::sync::Mutex; + +use super::thread_config_test_support::*; +use super::*; +use crate::thread_config_store::{RecordedConfig, RecordedValue}; + +const PLUS: &str = "Claude Agent (Plus)"; +const OTHER_ENTRY: &str = "Claude Agent"; +const PARENT: &str = "parent-1"; +const FORKED: &str = "forked-1"; + +/// What the agent reports for every session it opens — the parent's `session/load` and the +/// fork's `session/fork` alike — and keeps per session as values are set. +#[derive(Clone, Default)] +struct ForkScript { + config_options: Option<Vec<acp::SessionConfigOption>>, + modes: Option<acp::SessionModeState>, +} + +/// One request the agent received, with the session it names. +#[derive(Clone, Debug, PartialEq)] +enum Sent { + Config { + session: String, + config_id: String, + value: serde_json::Value, + }, + Mode { + session: String, + mode_id: String, + }, +} + +struct ForkConfigAgent { + connection: Rc<AcpConnection>, + project: Entity<Project>, + sent: Arc<Mutex<Vec<Sent>>>, + _keep_alive: Task<()>, +} + +impl ForkConfigAgent { + async fn connect( + script: ForkScript, + project: Entity<Project>, + cx: &mut gpui::TestAppContext, + ) -> Self { + let sent: Arc<Mutex<Vec<Sent>>> = Arc::default(); + // Per session, so setting a value on the parent never shows up in what the fork reports. + let options_by_session: Arc<Mutex<StdHashMap<String, Vec<acp::SessionConfigOption>>>> = + Arc::default(); + let script = Arc::new(script); + + let (client_transport, agent_transport) = agent_client_protocol::Channel::duplex(); + let agent_future = Agent + .v1() + .name("fork-config-agent") + .on_receive_request( + async move |req: acp::InitializeRequest, responder, _cx| { + responder.respond( + acp::InitializeResponse::new(req.protocol_version).agent_capabilities( + acp::AgentCapabilities::default() + .load_session(true) + .session_capabilities( + acp::SessionCapabilities::default() + .close(acp::SessionCloseCapabilities::new()) + .fork(acp::SessionForkCapabilities::new()), + ), + ), + ) + }, + agent_client_protocol::on_receive_request!(), + ) + .on_receive_request( + { + let script = script.clone(); + let options_by_session = options_by_session.clone(); + async move |req: acp::LoadSessionRequest, responder, _cx| { + if let Some(options) = script.config_options.clone() { + options_by_session + .lock() + .expect("agent options") + .insert(req.session_id.0.to_string(), options); + } + responder.respond( + acp::LoadSessionResponse::new() + .config_options(script.config_options.clone()) + .modes(script.modes.clone()), + ) + } + }, + agent_client_protocol::on_receive_request!(), + ) + .on_receive_request( + { + let script = script.clone(); + let options_by_session = options_by_session.clone(); + async move |_req: acp::ForkSessionRequest, responder, _cx| { + if let Some(options) = script.config_options.clone() { + options_by_session + .lock() + .expect("agent options") + .insert(FORKED.to_string(), options); + } + responder.respond( + acp::ForkSessionResponse::new(acp::SessionId::new(FORKED)) + .config_options(script.config_options.clone()) + .modes(script.modes.clone()), + ) + } + }, + agent_client_protocol::on_receive_request!(), + ) + .on_receive_request( + { + let sent = sent.clone(); + let options_by_session = options_by_session.clone(); + async move |req: acp::SetSessionConfigOptionRequest, responder, _cx| { + let params = serde_json::to_value(&req).expect("set config params"); + let session = req.session_id.0.to_string(); + let config_id = params["configId"].as_str().unwrap_or_default().to_string(); + let value = params["value"].clone(); + sent.lock().expect("sent").push(Sent::Config { + session: session.clone(), + config_id: config_id.clone(), + value: value.clone(), + }); + let list = { + let mut guard = options_by_session.lock().expect("agent options"); + let list = guard.entry(session).or_default(); + for option in list.iter_mut() { + if option.id.0.as_ref() != config_id { + continue; + } + match &mut option.kind { + acp::SessionConfigKind::Select(select) => { + if let Some(id) = value.as_str() { + select.current_value = + acp::SessionConfigValueId::new(id.to_string()); + } + } + acp::SessionConfigKind::Boolean(boolean) => { + if let Some(flag) = value.as_bool() { + boolean.current_value = flag; + } + } + #[allow(unreachable_patterns)] + _ => {} + } + } + list.clone() + }; + responder.respond(acp::SetSessionConfigOptionResponse::new(list)) + } + }, + agent_client_protocol::on_receive_request!(), + ) + .on_receive_request( + { + let sent = sent.clone(); + async move |req: acp::SetSessionModeRequest, responder, _cx| { + sent.lock().expect("sent").push(Sent::Mode { + session: req.session_id.0.to_string(), + mode_id: req.mode_id.0.to_string(), + }); + responder.respond(acp::SetSessionModeResponse::new()) + } + }, + agent_client_protocol::on_receive_request!(), + ) + .on_receive_request( + async move |_req: acp::CloseSessionRequest, responder, _cx| { + responder.respond(acp::CloseSessionResponse::new()) + }, + agent_client_protocol::on_receive_request!(), + ) + .on_receive_notification( + async move |_notif: acp::CancelNotification, _cx| Ok(()), + agent_client_protocol::on_receive_notification!(), + ) + .connect_to(agent_transport); + let agent_io_task = cx.background_spawn(agent_future); + + let sessions: Rc<RefCell<HashMap<acp_v2::SessionId, AcpSession>>> = + Rc::new(RefCell::new(HashMap::default())); + let client_session_list: Rc<RefCell<Option<Rc<AcpSessionList>>>> = + Rc::new(RefCell::new(None)); + let (dispatch_tx, dispatch_rx) = mpsc::unbounded::<ForegroundWork>(); + let (connection_tx, connection_rx) = futures::channel::oneshot::channel(); + let client_future = connect_client_future( + "zed-test", + client_transport, + dispatch_tx.clone(), + connection_tx, + futures::future::pending(), + ); + let client_io_task = cx.background_spawn(async move { + client_future.await.ok(); + }); + let client_conn: ConnectionTo<Agent> = connection_rx + .await + .expect("failed to receive ACP connection handle"); + let response = client_conn + .send_request(acp::InitializeRequest::new(ProtocolVersion::V1)) + .block_task() + .await + .expect("failed to initialize ACP connection"); + + let request_elicitations = cx.new(|_| ElicitationStore::default()); + let dispatch_context = ClientContext { + sessions: sessions.clone(), + pending_session_updates: None, + accepts_subagent_updates: false, + session_list: client_session_list.clone(), + request_elicitations: request_elicitations.clone(), + active_request_scopes: None, + }; + let dispatch_task = cx.spawn({ + let mut dispatch_rx = dispatch_rx; + move |cx| async move { + let mut cx = cx; + while let Some(work) = futures::StreamExt::next(&mut dispatch_rx).await { + work.run(&mut cx, &dispatch_context); + } + } + }); + let agent_server_store = + project.read_with(cx, |project, _| project.agent_server_store().downgrade()); + let mut connection = cx.update(|cx| { + AcpConnection::new_for_test( + client_conn, + sessions, + response.agent_capabilities, + false, + request_elicitations, + agent_server_store, + client_io_task, + dispatch_tx, + dispatch_task, + cx, + ) + }); + // Half of the record's key: the entry the parent was recorded under. + connection.id = AgentId::new(PLUS); + let keep_alive = cx.background_spawn(async move { + agent_io_task.await.ok(); + }); + + Self { + connection: Rc::new(connection), + project, + sent, + _keep_alive: keep_alive, + } + } + + /// The entry's `default_mode` and `default_config_options`. + fn set_defaults(&self, mode: Option<&str>, options: Vec<(&str, AgentConfigOptionValue)>) { + self.connection.defaults.set( + mode.map(|mode| acp_v2::SessionConfigValueId::new(mode.to_string())), + options + .into_iter() + .map(|(id, value)| (id.to_string(), value)) + .collect(), + ); + } + + fn work_dirs() -> PathList { + PathList::new(&[std::path::Path::new(WORK_DIR)]) + } + + /// The parent the fork is asked about: a session the connection already holds, opened as a + /// reopened thread is. + async fn open_parent(&self, cx: &mut gpui::TestAppContext) -> Entity<AcpThread> { + let thread = cx + .update(|cx| { + self.connection.clone().load_session( + acp_v2::SessionId::new(PARENT), + self.project.clone(), + Self::work_dirs(), + None, + cx, + ) + }) + .await + .expect("the parent session opens"); + cx.run_until_parked(); + thread + } + + async fn fork(&self, cx: &mut gpui::TestAppContext) -> Result<Entity<AcpThread>> { + let result = cx + .update(|cx| { + self.connection.clone().fork_session( + &acp_v2::SessionId::new(PARENT), + self.project.clone(), + Self::work_dirs(), + cx, + ) + }) + .await; + // The restoration (or the defaults) is detached; let every answer and record write land. + cx.run_until_parked(); + result + } + + /// `(config id, wire value)` of every `session/set_config_option` naming the fork, sorted. + fn fork_config_requests(&self) -> Vec<(String, serde_json::Value)> { + let mut requests: Vec<(String, serde_json::Value)> = self + .sent + .lock() + .expect("sent") + .iter() + .filter_map(|sent| match sent { + Sent::Config { + session, + config_id, + value, + } if session == FORKED => Some((config_id.clone(), value.clone())), + _ => None, + }) + .collect(); + requests.sort_by(|a, b| a.0.cmp(&b.0)); + requests + } + + /// Mode ids of every `session/set_mode` naming the fork, in order. + fn fork_mode_requests(&self) -> Vec<String> { + self.sent + .lock() + .expect("sent") + .iter() + .filter_map(|sent| match sent { + Sent::Mode { session, mode_id } if session == FORKED => Some(mode_id.clone()), + _ => None, + }) + .collect() + } + + /// The value Zeo holds for one of the fork's options: a JSON string or bool. + fn fork_option(&self, config_id: &str) -> Option<serde_json::Value> { + let sessions = self.connection.sessions.borrow(); + let session = sessions.get(&acp_v2::SessionId::new(FORKED))?; + let options = session.config_options.as_ref()?.config_options.borrow(); + let option = options + .iter() + .find(|option| option.config_id.0.as_ref() == config_id)?; + match &option.kind { + acp_v2::SessionConfigKind::Select(select) => { + Some(serde_json::json!(select.current_value.0.as_ref())) + } + acp_v2::SessionConfigKind::Boolean(boolean) => { + Some(serde_json::json!(boolean.current_value)) + } + _ => None, + } + } + + fn fork_mode(&self) -> Option<String> { + let sessions = self.connection.sessions.borrow(); + let session = sessions.get(&acp_v2::SessionId::new(FORKED))?; + let modes = session.session_modes.as_ref()?; + Some(modes.borrow().current_mode_id.0.to_string()) + } +} + +/// The Plus shape: model, mode and effort are selects, `fast` a session-scoped toggle. +fn plus_script(model: &str, mode: &str, effort: &str) -> ForkScript { + ForkScript { + config_options: Some(vec![ + select("model", model, &["sonnet", "opus", "haiku"]), + select("mode", mode, &["default", "plan", "auto"]), + select("effort", effort, &["low", "high"]), + boolean("fast", false), + ]), + modes: None, + } +} + +/// The entry defaults of the Plus entry: every one differs from what the parent recorded. +fn plus_defaults(agent: &ForkConfigAgent) { + agent.set_defaults( + None, + vec![ + ("model", AgentConfigOptionValue::ValueId("haiku".into())), + ("mode", AgentConfigOptionValue::ValueId("auto".into())), + ("effort", AgentConfigOptionValue::ValueId("low".into())), + ("fast", AgentConfigOptionValue::Boolean(true)), + ], + ); +} + +/// The parent's thread-local choice, covering every select the Plus shape offers. +fn parent_record() -> RecordedConfig { + recorded_config( + None, + vec![ + ("model", RecordedValue::Id("opus".into())), + ("mode", RecordedValue::Id("plan".into())), + ("effort", RecordedValue::Id("high".into())), + ], + ) +} + +fn plus_entry_defaults_on_the_wire() -> Vec<(String, serde_json::Value)> { + vec![ + wire("effort", serde_json::json!("low")), + wire("fast", serde_json::json!(true)), + wire("mode", serde_json::json!("auto")), + wire("model", serde_json::json!("haiku")), + ] +} + +// Hostile half (wrong collapse of "fork" into "new session") first — R6.5: the parent recorded +// model `opus`, mode `plan`, effort `high`; the agent reports `sonnet` / `default` / `high` for the +// fork; the entry defaults say `haiku` / `auto` / `low` / fast on. Only the recorded values that +// differ are sent, and not one default — `effort` is already the recorded value, so its differing +// default must not be sent either. +#[gpui::test] +async fn fork_config_entry_defaults_never_override_the_parents_record( + cx: &mut gpui::TestAppContext, +) { + let project = init(cx).await; + let agent = + ForkConfigAgent::connect(plus_script("sonnet", "default", "high"), project, cx).await; + plus_defaults(&agent); + record(PLUS, PARENT, parent_record(), cx).await; + let _parent = agent.open_parent(cx).await; + + let _forked = agent.fork(cx).await.expect("the agent forks"); + + assert_eq!( + agent.fork_config_requests(), + vec![ + wire("mode", serde_json::json!("plan")), + wire("model", serde_json::json!("opus")), + ], + "the fork receives the parent's differing recorded values and no entry default" + ); + assert_eq!(agent.fork_option("model"), Some(serde_json::json!("opus"))); + assert_eq!(agent.fork_option("mode"), Some(serde_json::json!("plan"))); + assert_eq!( + agent.fork_option("effort"), + Some(serde_json::json!("high")), + "the recorded effort stands over the `low` default" + ); + // A toggle is never recorded (`snapshot`), and with a record the defaults never apply — + // design.md: the fork opens the way a reopen opens a session. + assert_eq!( + agent.fork_option("fast"), + Some(serde_json::json!(false)), + "the `fast` default is not applied over a recorded parent" + ); + assert!( + agent.fork_mode_requests().is_empty(), + "the Plus shape carries the mode as an option; no session/set_mode" + ); +} + +// R6.5, legacy shape — an agent with session modes and no config options: the parent recorded +// `plan`, the entry's default mode is `acceptEdits`, the agent reports `default`. The fork gets one +// `session/set_mode` with `plan`, and never the default. +#[gpui::test] +async fn fork_config_parent_mode_replaces_the_entry_default_mode(cx: &mut gpui::TestAppContext) { + let project = init(cx).await; + let agent = ForkConfigAgent::connect( + ForkScript { + config_options: None, + modes: Some(modes("default", &["default", "plan", "acceptEdits"])), + }, + project, + cx, + ) + .await; + agent.set_defaults(Some("acceptEdits"), Vec::new()); + record(PLUS, PARENT, recorded_config(Some("plan"), Vec::new()), cx).await; + let _parent = agent.open_parent(cx).await; + + let _forked = agent.fork(cx).await.expect("the agent forks"); + + assert_eq!( + agent.fork_mode_requests(), + vec!["plan".to_string()], + "the fork receives the parent's recorded mode, not the entry's default mode" + ); + assert_eq!(agent.fork_mode(), Some("plan".to_string())); +} + +// Hostile half (a third element sharing the session id) — R6.6: the same session id recorded +// under ANOTHER entry is not the parent's record. The fork gets the entry defaults, as a parent +// with no record does. +#[gpui::test] +async fn fork_config_another_entrys_record_of_the_same_session_is_not_applied( + cx: &mut gpui::TestAppContext, +) { + let project = init(cx).await; + let agent = + ForkConfigAgent::connect(plus_script("sonnet", "default", "high"), project, cx).await; + plus_defaults(&agent); + record(OTHER_ENTRY, PARENT, parent_record(), cx).await; + let _parent = agent.open_parent(cx).await; + + let _forked = agent.fork(cx).await.expect("the agent forks"); + + assert_eq!( + agent.fork_config_requests(), + plus_entry_defaults_on_the_wire(), + "another entry's record is not the parent's: the fork gets the entry defaults" + ); + assert_eq!(agent.fork_option("model"), Some(serde_json::json!("haiku"))); +} + +// Benign half — R6.6: the parent has no record at all; the fork gets the entry defaults, exactly as +// a new session does. +#[gpui::test] +async fn fork_config_parent_without_a_record_gets_the_entry_defaults( + cx: &mut gpui::TestAppContext, +) { + let project = init(cx).await; + let agent = + ForkConfigAgent::connect(plus_script("sonnet", "default", "high"), project, cx).await; + plus_defaults(&agent); + let _parent = agent.open_parent(cx).await; + assert_eq!( + recorded(PLUS, PARENT, cx), + None, + "fixture: no parent record" + ); + + let _forked = agent.fork(cx).await.expect("the agent forks"); + + assert_eq!( + agent.fork_config_requests(), + plus_entry_defaults_on_the_wire() + ); + assert_eq!(agent.fork_option("model"), Some(serde_json::json!("haiku"))); + assert_eq!(agent.fork_option("mode"), Some(serde_json::json!("auto"))); +} + +// Hostile half (the fork's record never written) — the agent already reports the parent's values +// for the fork, so nothing is sent and nothing changes: the fork's session id must still hold the +// parent's record afterwards, and the parent's record is left as it was. +#[gpui::test] +async fn fork_config_the_fork_holds_the_parents_record_under_its_own_id( + cx: &mut gpui::TestAppContext, +) { + let project = init(cx).await; + let agent = ForkConfigAgent::connect(plus_script("opus", "plan", "high"), project, cx).await; + plus_defaults(&agent); + record(PLUS, PARENT, parent_record(), cx).await; + let _parent = agent.open_parent(cx).await; + + let _forked = agent.fork(cx).await.expect("the agent forks"); + + assert!( + agent.fork_config_requests().is_empty(), + "the agent already holds the parent's values: nothing to send, no default either" + ); + assert_eq!( + recorded(PLUS, FORKED, cx), + Some(parent_record()), + "the fork reopens with the parent's record" + ); + assert_eq!( + recorded(PLUS, PARENT, cx), + Some(parent_record()), + "the parent's record is untouched" + ); +} + +// R6.5 with a stale record — a recorded value the agent does not offer for the fork (`opus-legacy`, +// mode `ghost`) and an option it no longer has (`retired`) are skipped without failing the fork: +// those options keep the agent's value, no default fills the gap, and the offered recorded value +// (`effort: low`) is still sent. +#[gpui::test] +async fn fork_config_a_recorded_value_the_agent_does_not_offer_is_skipped( + cx: &mut gpui::TestAppContext, +) { + let project = init(cx).await; + let agent = + ForkConfigAgent::connect(plus_script("sonnet", "default", "high"), project, cx).await; + plus_defaults(&agent); + record( + PLUS, + PARENT, + recorded_config( + None, + vec![ + ("model", RecordedValue::Id("opus-legacy".into())), + ("mode", RecordedValue::Id("ghost".into())), + ("effort", RecordedValue::Id("low".into())), + ("retired", RecordedValue::Id("on".into())), + ], + ), + cx, + ) + .await; + let _parent = agent.open_parent(cx).await; + + let forked = agent.fork(cx).await; + + assert!( + forked.is_ok(), + "an unoffered recorded value does not fail the fork" + ); + assert!( + agent + .connection + .sessions + .borrow() + .contains_key(&acp_v2::SessionId::new(FORKED)), + "the fork is registered" + ); + assert_eq!( + agent.fork_config_requests(), + vec![wire("effort", serde_json::json!("low"))], + "only the offered recorded value is sent; no default replaces the skipped ones" + ); + assert_eq!( + agent.fork_option("model"), + Some(serde_json::json!("sonnet")), + "the agent's model stands; neither `opus-legacy` nor the `haiku` default" + ); + assert_eq!( + agent.fork_option("mode"), + Some(serde_json::json!("default")), + "the agent's mode stands; neither `ghost` nor the `auto` default" + ); +} diff --git a/crates/agent_servers/src/acp/v1/fork_session_tests.rs b/crates/agent_servers/src/acp/v1/fork_session_tests.rs new file mode 100644 index 0000000..adaf7ec --- /dev/null +++ b/crates/agent_servers/src/acp/v1/fork_session_tests.rs @@ -0,0 +1,445 @@ +//! Story 039, task 3.2 — `session/fork` over ACP v1 (R6.1, R6.2, R6.3; Q4, Q5, Q7): the real +//! `AcpConnection` against a scripted agent on the SDK's in-memory channel, with the production +//! client handler set, so the fork request travels exactly as it does to the Plus adapter. +//! +//! Materialize with, in `crates/agent_servers/src/acp/v1.rs`, beside `mod round_trip_tests;`: +//! #[cfg(test)] +//! mod fork_session_tests; +//! Filter: `cargo test -p agent_servers fork` (paths `acp::v1::fork_session_tests::…`). +//! +//! Contract (design.md *Fork over ACP v1*): +//! - `supports_fork_session()` is `sessionCapabilities.fork` (R6.1, R6.2); +//! - `SessionDirectories::into_fork_session_request(self, session_id: acp_v2::SessionId, +//! mcp_servers: Vec<acp::McpServer>) -> acp::ForkSessionRequest`; +//! - `fork_session(parent, project, work_dirs, cx)`: no capability → an error and no request; +//! otherwise ONE `session/fork` with the parent id, the work dirs (first = cwd, the rest = +//! additional directories) and `mcp_servers_for_project`, and the thread for the answered id +//! registered as `new_session` registers one and marked `mark_forked_from(parent)` (R6.3, +//! task 3.3); a refusal registers nothing; a caller gone +//! before the answer leaves the answered session closed (copying `new_session`). +//! +//! Hostile halves first: the registered thread is the ANSWERED id's, never the parent's (wrong +//! collapse); asked for again by id it is the same thread, opened without a `session/load` (wrong +//! split — design.md's Decision depends on it). +//! +//! Gap, named: the fixture project has no context servers, so the MCP list compared is empty. The +//! unit test below carries a non-empty list through the request builder. + +use std::sync::Mutex; +use std::sync::atomic::{AtomicUsize, Ordering}; + +use super::*; +use futures::StreamExt as _; + +const CWD: &str = "/a"; +const EXTRA: &str = "/b"; +const PARENT: &str = "parent-1"; +const FORKED: &str = "forked-1"; + +#[derive(Clone, Copy)] +enum ForkAnswer { + /// Answers `session/fork` at once with `FORKED`. + Forked, + /// Answers with an internal error. + Refused, + /// Answers with `FORKED` only after `release`. + Held, +} + +struct ForkAgent { + connection: Rc<AcpConnection>, + project: Entity<Project>, + forks: Arc<Mutex<Vec<acp::ForkSessionRequest>>>, + loads: Arc<AtomicUsize>, + closes: Arc<Mutex<Vec<acp::SessionId>>>, + release: async_channel::Sender<()>, + _keep_alive: Task<()>, +} + +impl ForkAgent { + async fn connect( + advertises_fork: bool, + answer: ForkAnswer, + cx: &mut gpui::TestAppContext, + ) -> Self { + cx.update(|cx| { + let store = settings::SettingsStore::test(cx); + cx.set_global(store); + cx.set_global(db::AppDatabase::test_new()); + }); + let fs = fs::FakeFs::new(cx.executor()); + fs.insert_tree("/", serde_json::json!({ "a": {}, "b": {} })) + .await; + let project = project::Project::test(fs, [std::path::Path::new(CWD)], cx).await; + + let forks: Arc<Mutex<Vec<acp::ForkSessionRequest>>> = Arc::default(); + let loads = Arc::new(AtomicUsize::new(0)); + let closes: Arc<Mutex<Vec<acp::SessionId>>> = Arc::default(); + let (release, released) = async_channel::bounded::<()>(1); + + let (client_transport, agent_transport) = agent_client_protocol::Channel::duplex(); + let agent_future = Agent + .v1() + .name("fork-agent") + .on_receive_request( + async move |req: acp::InitializeRequest, responder, _cx| { + let session_capabilities = acp::SessionCapabilities::default() + .close(acp::SessionCloseCapabilities::new()) + .additional_directories( + acp::SessionAdditionalDirectoriesCapabilities::new(), + ); + let session_capabilities = if advertises_fork { + session_capabilities.fork(acp::SessionForkCapabilities::new()) + } else { + session_capabilities + }; + responder.respond( + acp::InitializeResponse::new(req.protocol_version).agent_capabilities( + acp::AgentCapabilities::default() + .load_session(true) + .session_capabilities(session_capabilities), + ), + ) + }, + agent_client_protocol::on_receive_request!(), + ) + .on_receive_request( + { + let loads = loads.clone(); + async move |_req: acp::LoadSessionRequest, responder, _cx| { + loads.fetch_add(1, Ordering::SeqCst); + responder.respond(acp::LoadSessionResponse::new()) + } + }, + agent_client_protocol::on_receive_request!(), + ) + .on_receive_request( + { + let forks = forks.clone(); + async move |req: acp::ForkSessionRequest, responder, _cx| { + forks.lock().expect("fork requests").push(req); + match answer { + ForkAnswer::Forked => responder.respond(acp::ForkSessionResponse::new( + acp::SessionId::new(FORKED), + )), + ForkAnswer::Refused => responder + .respond_with_error(agent_client_protocol::Error::internal_error()), + ForkAnswer::Held => { + released.recv().await.ok(); + responder.respond(acp::ForkSessionResponse::new( + acp::SessionId::new(FORKED), + )) + } + } + } + }, + agent_client_protocol::on_receive_request!(), + ) + .on_receive_request( + { + let closes = closes.clone(); + async move |req: acp::CloseSessionRequest, responder, _cx| { + closes.lock().expect("close requests").push(req.session_id); + responder.respond(acp::CloseSessionResponse::new()) + } + }, + agent_client_protocol::on_receive_request!(), + ) + .on_receive_notification( + async move |_notif: acp::CancelNotification, _cx| Ok(()), + agent_client_protocol::on_receive_notification!(), + ) + .connect_to(agent_transport); + let agent_io_task = cx.background_spawn(agent_future); + + let sessions: Rc<RefCell<HashMap<acp_v2::SessionId, AcpSession>>> = + Rc::new(RefCell::new(HashMap::default())); + let client_session_list: Rc<RefCell<Option<Rc<AcpSessionList>>>> = + Rc::new(RefCell::new(None)); + let (dispatch_tx, dispatch_rx) = mpsc::unbounded::<ForegroundWork>(); + let (connection_tx, connection_rx) = futures::channel::oneshot::channel(); + let client_future = connect_client_future( + "zed-test", + client_transport, + dispatch_tx.clone(), + connection_tx, + futures::future::pending(), + ); + let client_io_task = cx.background_spawn(async move { + client_future.await.ok(); + }); + let client_conn: ConnectionTo<Agent> = connection_rx + .await + .expect("failed to receive ACP connection handle"); + let response = client_conn + .send_request(acp::InitializeRequest::new(ProtocolVersion::V1)) + .block_task() + .await + .expect("failed to initialize ACP connection"); + + let request_elicitations = cx.new(|_| ElicitationStore::default()); + let dispatch_context = ClientContext { + sessions: sessions.clone(), + pending_session_updates: None, + accepts_subagent_updates: false, + session_list: client_session_list.clone(), + request_elicitations: request_elicitations.clone(), + active_request_scopes: None, + }; + let dispatch_task = cx.spawn({ + let mut dispatch_rx = dispatch_rx; + move |cx| async move { + let mut cx = cx; + while let Some(work) = dispatch_rx.next().await { + work.run(&mut cx, &dispatch_context); + } + } + }); + let agent_server_store = + project.read_with(cx, |project, _| project.agent_server_store().downgrade()); + let connection = cx.update(|cx| { + AcpConnection::new_for_test( + client_conn, + sessions, + response.agent_capabilities, + false, + request_elicitations, + agent_server_store, + client_io_task, + dispatch_tx, + dispatch_task, + cx, + ) + }); + let keep_alive = cx.background_spawn(async move { + agent_io_task.await.ok(); + }); + + Self { + connection: Rc::new(connection), + project, + forks, + loads, + closes, + release, + _keep_alive: keep_alive, + } + } + + fn work_dirs() -> PathList { + PathList::new(&[std::path::Path::new(CWD), std::path::Path::new(EXTRA)]) + } + + /// The parent the fork is asked about: a session the connection already holds. + async fn open_parent(&self, cx: &mut gpui::TestAppContext) -> Entity<AcpThread> { + let thread = cx + .update(|cx| { + self.connection.clone().load_session( + acp_v2::SessionId::new(PARENT), + self.project.clone(), + Self::work_dirs(), + None, + cx, + ) + }) + .await + .expect("the parent session opens"); + cx.run_until_parked(); + self.loads.store(0, Ordering::SeqCst); + thread + } + + fn fork(&self, cx: &mut gpui::TestAppContext) -> Task<Result<Entity<AcpThread>>> { + cx.update(|cx| { + self.connection.clone().fork_session( + &acp_v2::SessionId::new(PARENT), + self.project.clone(), + Self::work_dirs(), + cx, + ) + }) + } + + fn fork_requests(&self) -> Vec<acp::ForkSessionRequest> { + self.forks.lock().expect("fork requests").clone() + } + + fn registered(&self, id: &str) -> bool { + self.connection + .sessions + .borrow() + .contains_key(&acp_v2::SessionId::new(id)) + } +} + +fn session_of(thread: &Entity<AcpThread>, cx: &gpui::TestAppContext) -> acp_v2::SessionId { + thread.read_with(cx, |thread, _| thread.session_id().clone()) +} + +// The builder carries all four fields, a non-empty MCP list included (design.md, Testing +// Strategy: unit). +#[test] +fn fork_request_carries_parent_id_directories_and_mcp_servers() { + let servers = vec![acp::McpServer::Stdio(acp::McpServerStdio::new( + "notes", + "/usr/bin/notes-mcp", + ))]; + let directories = SessionDirectories { + cwd: PathBuf::from(CWD), + additional_directories: vec![PathBuf::from(EXTRA)], + }; + + let request = + directories.into_fork_session_request(acp_v2::SessionId::new(PARENT), servers.clone()); + + assert_eq!(request.session_id, acp::SessionId::new(PARENT)); + assert_eq!(request.cwd, PathBuf::from(CWD)); + assert_eq!(request.additional_directories, vec![PathBuf::from(EXTRA)]); + assert_eq!(request.mcp_servers, servers); +} + +// Hostile half (wrong collapse) first — R6.1, R6.3: with the capability, ONE request naming the +// PARENT, its directories and the project's MCP servers; the thread returned and registered is +// the ANSWERED id's, a new thread beside the parent's, not the parent's. +#[gpui::test] +async fn fork_sends_one_request_and_registers_the_answered_session(cx: &mut gpui::TestAppContext) { + let agent = ForkAgent::connect(true, ForkAnswer::Forked, cx).await; + assert!(agent.connection.supports_fork_session(), "R6.1"); + let parent = agent.open_parent(cx).await; + + let forked = agent.fork(cx).await.expect("the agent forks"); + cx.run_until_parked(); + + let requests = agent.fork_requests(); + assert_eq!(requests.len(), 1, "exactly one session/fork"); + let request = &requests[0]; + assert_eq!( + request.session_id, + acp::SessionId::new(PARENT), + "names the parent" + ); + assert_eq!( + request.cwd, + PathBuf::from(CWD), + "the parent's working directory" + ); + assert_eq!( + request.additional_directories, + vec![PathBuf::from(EXTRA)], + "the parent's additional directories" + ); + let expected_servers = cx.update(|cx| mcp_servers_for_project(&agent.project, cx)); + assert_eq!( + request.mcp_servers, expected_servers, + "the project's MCP servers" + ); + + assert_eq!(session_of(&forked, cx), acp_v2::SessionId::new(FORKED)); + assert_ne!(forked, parent, "a new thread, not the parent's"); + assert!(agent.registered(FORKED), "registered under the answered id"); + assert_eq!( + forked.read_with(cx, |thread, _| thread.forked_from().cloned()), + Some(acp_v2::SessionId::new(PARENT)), + "marked forked from the parent (task 3.3), so it is not a draft while empty" + ); + assert!(!forked.read_with(cx, |thread, _| thread.is_draft_thread())); + assert_eq!( + parent.read_with(cx, |thread, _| thread.forked_from().cloned()), + None, + "the parent carries no mark" + ); + assert!(agent.registered(PARENT), "the parent stays registered"); + assert_eq!(session_of(&parent, cx), acp_v2::SessionId::new(PARENT)); +} + +// Hostile half (wrong split) — the registered fork, opened again by id, is the same thread and no +// `session/load` is sent: nothing can be replayed into the view that attaches to it. +#[gpui::test] +async fn fork_registered_thread_reopens_without_a_load_request(cx: &mut gpui::TestAppContext) { + let agent = ForkAgent::connect(true, ForkAnswer::Forked, cx).await; + let _parent = agent.open_parent(cx).await; + let forked = agent.fork(cx).await.expect("the agent forks"); + cx.run_until_parked(); + + let reopened = cx + .update(|cx| { + agent.connection.clone().load_session( + acp_v2::SessionId::new(FORKED), + agent.project.clone(), + ForkAgent::work_dirs(), + None, + cx, + ) + }) + .await + .expect("the forked session opens"); + cx.run_until_parked(); + + assert_eq!(reopened, forked, "the registered thread itself"); + assert_eq!( + agent.loads.load(Ordering::SeqCst), + 0, + "no session/load was sent" + ); +} + +// R6.2 — without the capability: reported as unsupported, refused, and nothing reaches the agent. +#[gpui::test] +async fn fork_without_the_capability_refuses_and_sends_nothing(cx: &mut gpui::TestAppContext) { + let agent = ForkAgent::connect(false, ForkAnswer::Forked, cx).await; + assert!(!agent.connection.supports_fork_session(), "R6.2"); + let _parent = agent.open_parent(cx).await; + + let result = agent.fork(cx).await; + cx.run_until_parked(); + + assert!(result.is_err(), "refused"); + assert!(agent.fork_requests().is_empty(), "no session/fork was sent"); + assert!(!agent.registered(FORKED)); +} + +// A refused fork returns the error and registers nothing — no session, no pending session. +#[gpui::test] +async fn fork_refused_by_the_agent_registers_nothing(cx: &mut gpui::TestAppContext) { + let agent = ForkAgent::connect(true, ForkAnswer::Refused, cx).await; + let _parent = agent.open_parent(cx).await; + let sessions_before = agent.connection.sessions.borrow().len(); + + let result = agent.fork(cx).await; + cx.run_until_parked(); + + assert!(result.is_err(), "the agent's refusal is an error"); + assert_eq!(agent.fork_requests().len(), 1, "it was asked once"); + assert_eq!(agent.connection.sessions.borrow().len(), sessions_before); + assert!(agent.connection.pending_sessions.borrow().is_empty()); + assert!( + agent.closes.lock().expect("closes").is_empty(), + "nothing to close" + ); +} + +// A caller gone before the answer: the answered session is not registered and is closed, as +// `new_session` closes an abandoned one (design.md, *Fork over ACP v1*). +#[gpui::test] +async fn fork_abandoned_before_the_answer_closes_the_answered_session( + cx: &mut gpui::TestAppContext, +) { + let agent = ForkAgent::connect(true, ForkAnswer::Held, cx).await; + let _parent = agent.open_parent(cx).await; + + let pending = agent.fork(cx); + cx.run_until_parked(); + assert_eq!(agent.fork_requests().len(), 1, "the request is in flight"); + drop(pending); + cx.run_until_parked(); + + agent.release.try_send(()).expect("release the answer"); + cx.run_until_parked(); + + assert!(!agent.registered(FORKED), "nobody holds the fork"); + assert_eq!( + agent.closes.lock().expect("closes").clone(), + vec![acp::SessionId::new(FORKED)], + "the answered session is closed, the parent is not" + ); +} diff --git a/crates/agent_ui/src/agent_panel.rs b/crates/agent_ui/src/agent_panel.rs index 902ef3e..7e3846f 100644 --- a/crates/agent_ui/src/agent_panel.rs +++ b/crates/agent_ui/src/agent_panel.rs @@ -1710,6 +1710,10 @@ impl AgentPanel { &self.connection_store } + pub(crate) fn panel_workspace(&self) -> &WeakEntity<Workspace> { + &self.workspace + } + pub fn selected_agent(&self, cx: &App) -> Agent { if self.project.read(cx).is_via_collab() { Agent::NativeAgent @@ -2414,8 +2418,10 @@ impl AgentPanel { } /// Opens a new thread for `agent` with `text` in its composer -- never sent: - /// the operator reviews it and sends it. An agent that is no longer - /// configured opens nothing and says so. `source_session` is the session whose + /// the operator reviews it and sends it -- and returns its conversation. An + /// agent that is no longer configured opens nothing, says so, and returns + /// `None`; a panel with no open project also returns `None`, silently, as + /// `external_thread` does. `source_session` is the session whose /// message offered the command -- a subagent's, when the click came from one, /// which the panel's active thread (always the root) would misreport. pub fn new_thread_with_prefilled_prompt( @@ -2423,17 +2429,19 @@ impl AgentPanel { agent: Agent, source_session: acp_v2::SessionId, text: String, + focus: bool, window: &mut Window, cx: &mut Context<Self>, - ) { + ) -> Option<Entity<ConversationView>> { if !self.ensure_agent_available(&agent, &source_session, "prefill", cx) { - return; + return None; } let agent_id = agent.id(); log::info!( "opening a prefilled thread: agent {agent_id}, source session {source_session}, command length {}", text.len() ); + let before = self.active_conversation_view().cloned(); self.external_thread( Some(agent), None, @@ -2443,11 +2451,14 @@ impl AgentPanel { blocks: vec![acp_v2::ContentBlock::Text(acp_v2::TextContent::new(text))], auto_submit: false, }), - true, + focus, AgentThreadSource::AgentPanel, window, cx, ); + self.active_conversation_view() + .filter(|opened| Some(*opened) != before.as_ref()) + .cloned() } fn set_selected_agent_and_persist(&mut self, agent: Agent, cx: &mut Context<Self>) { @@ -3466,7 +3477,7 @@ impl AgentPanel { .and_then(|workspace| terminal_view::default_working_directory(workspace.read(cx), cx)) } - fn has_open_project(&self, cx: &App) -> bool { + pub(crate) fn has_open_project(&self, cx: &App) -> bool { self.project.read(cx).visible_worktrees(cx).next().is_some() } @@ -3552,6 +3563,7 @@ impl AgentPanel { || draft.read(cx).has_pending_selections() || !draft_is_active { + crate::sub_thread::drop_link_of_reused_draft(draft, has_editor_content, cx); return draft.clone(); } @@ -5009,6 +5021,20 @@ impl AgentPanel { } } + /// [`Self::set_base_view`] for a thread opened outside this module (story 039's fork), which + /// cannot name the private `BaseView`; returns the conversation it shows. + pub(crate) fn show_agent_thread( + &mut self, + thread: AgentThread, + focus: bool, + window: &mut Window, + cx: &mut Context<Self>, + ) -> Entity<ConversationView> { + let conversation_view = thread.conversation_view.clone(); + self.set_base_view(thread.into(), focus, window, cx); + conversation_view + } + fn set_base_view( &mut self, new_view: BaseView, @@ -5296,7 +5322,7 @@ impl AgentPanel { /// handler, clipboard import) to mint a [`ThreadId`] + seed metadata /// so they can route through [`create_agent_thread_with_server`] and /// this entry can be deleted. - fn create_agent_thread_with_server_for_external_session( + pub(crate) fn create_agent_thread_with_server_for_external_session( &mut self, agent: Agent, server_override: Option<Rc<dyn AgentServer>>, diff --git a/crates/agent_ui/src/agent_panel/tests/prefilled_prompt_tests.rs b/crates/agent_ui/src/agent_panel/tests/prefilled_prompt_tests.rs index df293c2..a185c49 100644 --- a/crates/agent_ui/src/agent_panel/tests/prefilled_prompt_tests.rs +++ b/crates/agent_ui/src/agent_panel/tests/prefilled_prompt_tests.rs @@ -188,6 +188,7 @@ async fn test_prefilled_prompt_opens_a_new_thread_with_the_text_unsent(cx: &mut Agent::Stub, source_session.clone(), COMMAND.to_string(), + true, window, cx, ); @@ -233,6 +234,7 @@ async fn test_prefilled_prompt_keeps_the_text_verbatim(cx: &mut TestAppContext) Agent::Stub, source_session.clone(), text.to_string(), + true, window, cx, ); @@ -266,6 +268,7 @@ async fn test_prefilled_prompt_for_an_unavailable_agent_toasts_and_opens_nothing retired, source_session.clone(), COMMAND.to_string(), + true, window, cx, ); diff --git a/crates/agent_ui/src/agent_panel/tests/run_prompt_tests.rs b/crates/agent_ui/src/agent_panel/tests/run_prompt_tests.rs index b448a4a..895d868 100644 --- a/crates/agent_ui/src/agent_panel/tests/run_prompt_tests.rs +++ b/crates/agent_ui/src/agent_panel/tests/run_prompt_tests.rs @@ -234,6 +234,7 @@ async fn test_run_prompt_leaves_the_prefill_path_unsent(cx: &mut TestAppContext) Agent::Stub, source_session, COMMAND.to_string(), + true, window, cx, ); diff --git a/crates/agent_ui/src/agent_panel/tests/run_prompt_validation_tests.rs b/crates/agent_ui/src/agent_panel/tests/run_prompt_validation_tests.rs index 16a5e5d..5b6acc0 100644 --- a/crates/agent_ui/src/agent_panel/tests/run_prompt_validation_tests.rs +++ b/crates/agent_ui/src/agent_panel/tests/run_prompt_validation_tests.rs @@ -193,6 +193,7 @@ async fn test_prefilled_command_sent_from_the_composer_is_refused(cx: &mut TestA Agent::Stub, source_session, COMMAND.to_string(), + true, window, cx, ); @@ -324,6 +325,7 @@ async fn test_threads_opened_after_run_are_still_checked(cx: &mut TestAppContext Agent::Stub, source_session, SECOND_COMMAND.to_string(), + true, window, cx, ); diff --git a/crates/agent_ui/src/agent_ui.rs b/crates/agent_ui/src/agent_ui.rs index ff0b0d8..62939b4 100644 --- a/crates/agent_ui/src/agent_ui.rs +++ b/crates/agent_ui/src/agent_ui.rs @@ -27,6 +27,7 @@ mod model_selector; mod model_selector_popover; mod profile_selector; mod set_as_default; +pub mod sub_thread; mod task_label; mod tasks_card; mod terminal_codegen; diff --git a/crates/agent_ui/src/conversation_view.rs b/crates/agent_ui/src/conversation_view.rs index 964fb48..3477ff5 100644 --- a/crates/agent_ui/src/conversation_view.rs +++ b/crates/agent_ui/src/conversation_view.rs @@ -112,6 +112,7 @@ pub(crate) mod elicitation; mod message_queue; mod sent_history; mod session_stats; +mod sub_thread_header; mod subagent_card; mod thread_search_bar; mod thread_view; @@ -863,6 +864,7 @@ pub struct ConversationView { /// Shared with the child [`ThreadView`] when one is constructed. pub(crate) code_span_resolver: AgentCodeSpanResolver, request_elicitation_form_states: HashMap<ElicitationEntryId, ElicitationFormState>, + pub(crate) sub_thread: crate::sub_thread::SubThreadViewState, _subscriptions: Vec<Subscription>, } @@ -1031,6 +1033,7 @@ enum ServerState { LoadError { error: LoadError, }, + SubThreadUnavailable, Connected(ConnectedServerState), } @@ -1179,6 +1182,8 @@ impl ConversationView { } })); + subscriptions.extend(crate::sub_thread::observe_sub_thread_stores(cx)); + cx.on_release(|this, cx| { this.request_elicitation_form_states.clear(); if let Some(connected) = this.as_connected() { @@ -1230,14 +1235,17 @@ impl ConversationView { draft_prompt_persist_task: None, code_span_resolver, request_elicitation_form_states: HashMap::default(), + sub_thread: Default::default(), _subscriptions: subscriptions, focus_handle: cx.focus_handle(), } } fn set_server_state(&mut self, state: ServerState, cx: &mut Context<Self>) { - if matches!(&state, ServerState::LoadError { .. }) - && let Some(view) = self.root_thread_view() + if matches!( + &state, + ServerState::LoadError { .. } | ServerState::SubThreadUnavailable + ) && let Some(view) = self.root_thread_view() && let Some(recovery) = view.read(cx).snapshot_recovery(cx) { self.draft_prompt_persist_task.take(); @@ -1306,7 +1314,8 @@ impl ConversationView { ServerState::Loading { connection: None, .. } - | ServerState::LoadError { .. } => None, + | ServerState::LoadError { .. } + | ServerState::SubThreadUnavailable => None, } } @@ -1605,6 +1614,9 @@ impl ConversationView { this.saved_submissions.extend(recovery.submissions); } } + Err(err) if this.is_never_used_fork(&err, cx) => { + this.show_sub_thread_unavailable(window, cx) + } Err(err) => { this.handle_load_error( LoadError::Other(err.to_string().into()), @@ -1918,7 +1930,7 @@ impl ConversationView { // when agent.connect() fails during loading), retry loading the thread. // This handles the case where a thread is restored before authentication completes. let should_retry = match &self.server_state { - ServerState::Loading { .. } => false, + ServerState::Loading { .. } | ServerState::SubThreadUnavailable => false, ServerState::LoadError { .. } => true, ServerState::Connected(connected) => { connected.auth_state.is_ok() && connected.has_thread_error(cx) @@ -1971,6 +1983,7 @@ impl ConversationView { LoadError::Exited { .. } => format!("{} Exited", self.agent.agent_id()).into(), LoadError::Other(_) => format!("Error Loading {}", self.agent.agent_id()).into(), }, + ServerState::SubThreadUnavailable => self.sub_thread_unavailable_title(cx), } } @@ -4146,6 +4159,7 @@ impl Render for ConversationView { .justify_end() .child(self.render_load_error(e, window, cx)) .into_any(), + ServerState::SubThreadUnavailable => self.render_sub_thread_unavailable(cx), ServerState::Connected(ConnectedServerState { connection, auth_state: @@ -8492,6 +8506,7 @@ pub(crate) mod tests { ServerState::Loading { .. } => "Loading (stuck!)", ServerState::LoadError { .. } => "LoadError (wrong variant)", ServerState::Connected(_) => "Connected", + ServerState::SubThreadUnavailable => "SubThreadUnavailable", } ), } diff --git a/crates/agent_ui/src/conversation_view/sub_thread_header.rs b/crates/agent_ui/src/conversation_view/sub_thread_header.rs new file mode 100644 index 0000000..36d03a7 --- /dev/null +++ b/crates/agent_ui/src/conversation_view/sub_thread_header.rs @@ -0,0 +1,263 @@ +//! Story 039 (R4): "Sub-thread of: <parent>" and the inherited band above a sub-thread's +//! messages. Re-rendered by `ConversationView`'s observation of both stores +//! (`crate::sub_thread::observe_sub_thread_stores`): `ThreadView` observes neither. + +use gpui::{AnyElement, App, Context, Div, Focusable, SharedString, WeakEntity}; +use ui::prelude::*; +use workspace::Workspace; + +use super::thread_view::ThreadView; +use super::{ConversationView, ServerState}; +use crate::thread_metadata_store::{ThreadId, ThreadMetadata, ThreadMetadataStore}; +use crate::thread_organization::{ForkKind, ThreadOrganizationStore}; +use crate::{Agent, AgentPanel, AgentThreadSource}; + +/// `None` when `thread_id` has no parent link (R4.7). A parent whose metadata is gone was +/// deleted: the organization store keeps a child's link to it (R4.3). +pub fn render_sub_thread_header( + thread_id: ThreadId, + inherited: Option<usize>, + workspace: WeakEntity<Workspace>, + cx: &App, +) -> Option<AnyElement> { + let (parent_id, kind) = ThreadOrganizationStore::try_global(cx)? + .read(cx) + .parent(thread_id)?; + let parent = ThreadMetadataStore::try_global(cx) + .and_then(|store| store.read(cx).entry(parent_id).cloned()); + let Some(parent) = parent else { + return Some( + header_frame(cx) + .child(deleted_parent_header()) + .into_any_element(), + ); + }; + let title = parent.display_title(); + let band = (kind == ForkKind::WithContext).then(|| inherited_band(inherited, &title)); + Some( + header_frame(cx) + .child(parent_header(parent, title, workspace)) + .children(band) + .into_any_element(), + ) +} + +fn header_frame(cx: &App) -> Div { + v_flex() + .px_4() + .py_1() + .gap_0p5() + .border_b_1() + .border_color(cx.theme().colors().border_variant) +} + +fn parent_header( + parent: ThreadMetadata, + title: SharedString, + workspace: WeakEntity<Workspace>, +) -> impl IntoElement { + let header_text = format!("Sub-thread of: {title}"); + h_flex() + .gap_1() + .debug_selector(move || format!("SUB_THREAD_HEADER-{header_text}")) + .child( + Label::new("Sub-thread of:") + .size(LabelSize::Small) + .color(Color::Muted), + ) + .child( + div() + .debug_selector(|| "SUB_THREAD_PARENT".to_string()) + .child( + Button::new("sub-thread-parent", title) + .label_size(LabelSize::Small) + .tab_index(0_isize) + .on_click(move |_, window, cx| { + open_parent(&parent, &workspace, window, cx) + }), + ), + ) +} + +fn deleted_parent_header() -> impl IntoElement { + const TEXT: &str = "Sub-thread of a deleted thread"; + div() + .debug_selector(|| format!("SUB_THREAD_HEADER-{TEXT}")) + .child(Label::new(TEXT).size(LabelSize::Small).color(Color::Muted)) +} + +/// R4.4 / R4.5: the count is known only for a fork made in this session. +fn inherited_band(inherited: Option<usize>, title: &SharedString) -> Div { + let text = match inherited { + Some(count) => format!("Inherited {count} messages from {title}"), + None => format!("Inherited the conversation of {title}"), + }; + let selector = format!("SUB_THREAD_BAND-{text}"); + div().debug_selector(move || selector).child( + Label::new(text) + .size(LabelSize::Small) + .color(Color::Muted) + .truncate(), + ) +} + +fn open_parent( + parent: &ThreadMetadata, + workspace: &WeakEntity<Workspace>, + window: &mut Window, + cx: &mut App, +) { + let panel = workspace + .upgrade() + .and_then(|workspace| workspace.read(cx).panel::<AgentPanel>(cx)); + let Some(panel) = panel else { + log::warn!( + "zeo_sub_thread event=open_parent_failed parent={} reason=no_agent_panel", + parent.thread_id.to_key_string() + ); + return; + }; + panel.update(cx, |panel, cx| { + panel.load_agent_thread( + Agent::from(parent.agent_id.clone()), + parent.thread_id, + Some(parent.folder_paths().clone()), + parent.title.clone(), + true, + AgentThreadSource::AgentPanel, + window, + cx, + ) + }); +} + +impl ThreadView { + /// The one hook `ThreadView`'s render calls, above the entries. Only the root thread of a + /// conversation is a sub-thread; a subagent's view inside it is not. + pub(super) fn sub_thread_header(&self, cx: &App) -> Option<AnyElement> { + if self.parent_session_id.is_some() { + return None; + } + let inherited = self + .server_view + .upgrade() + .and_then(|view| view.read(cx).sub_thread.inherited_messages()); + render_sub_thread_header(self.root_thread_id, inherited, self.workspace.clone(), cx) + } +} + +// R4.9, word for word. +const NEVER_USED_FORK_TEXT: &str = "This sub-thread was never used and can't be reopened"; + +/// A reopen through the real ACP connection fails inside `open_or_create_session`'s shared task, +/// which holds the error as `Arc<anyhow::Error>`; its caller re-wraps that `Arc` with `anyhow!`, +/// and an `Arc<anyhow::Error>` is no `std::error::Error`, so anyhow keeps it as an ad-hoc message +/// and a direct downcast to `acp::Error` misses it. Found by the task 8.1 live proof, 2026-10-23: +/// the GPUI tests' fake connection skips that task. +fn is_sub_thread_resource_not_found(error: &anyhow::Error) -> bool { + let acp_error = error + .downcast_ref::<agent_client_protocol::Error>() + .or_else(|| { + error + .downcast_ref::<std::sync::Arc<anyhow::Error>>() + .and_then(|shared| shared.downcast_ref::<agent_client_protocol::Error>()) + }); + acp_error + .is_some_and(|error| error.code == agent_client_protocol::ErrorCode::RESOURCE_NOT_FOUND) +} + +#[cfg(test)] +mod resource_not_found_tests { + use super::is_sub_thread_resource_not_found; + use agent_client_protocol::Error as AcpError; + use std::sync::Arc; + + #[test] + fn found_directly_and_through_the_shared_load_task() { + let direct = anyhow::anyhow!(AcpError::new( + agent_client_protocol::ErrorCode::RESOURCE_NOT_FOUND, + "Resource not found", + )); + assert!(is_sub_thread_resource_not_found(&direct)); + // The shape `open_or_create_session` hands back: `anyhow!(Arc<anyhow::Error>)`. + let shared = anyhow::anyhow!(Arc::new(anyhow::anyhow!(AcpError::new( + agent_client_protocol::ErrorCode::RESOURCE_NOT_FOUND, + "Resource not found", + )))); + assert!(is_sub_thread_resource_not_found(&shared)); + } + + #[test] + fn other_codes_and_plain_messages_are_not() { + let internal = anyhow::anyhow!(Arc::new(anyhow::anyhow!(AcpError::internal_error()))); + assert!(!is_sub_thread_resource_not_found(&internal)); + let message = anyhow::anyhow!("Resource not found"); + assert!(!is_sub_thread_resource_not_found(&message)); + } +} + +impl ConversationView { + /// R4.9. The adapter reloads a fork by resuming its SDK session, and a fork nobody prompted + /// left no transcript, so after a restart the reload answers resource-not-found (the 2026-10-21 + /// probe, claude-agent-acp-plus 0.30.0). Only a with-context sub-thread lost something to it: a + /// clean one, an unlinked thread or another code keeps upstream's load error. `map_acp_error` + /// keeps the `acp::Error` inside the `anyhow::Error`, under any context it adds. + pub(super) fn is_never_used_fork(&self, error: &anyhow::Error, cx: &App) -> bool { + is_sub_thread_resource_not_found(error) + && ThreadOrganizationStore::try_global(cx) + .and_then(|store| store.read(cx).parent(self.thread_id)) + .is_some_and(|(_, kind)| kind == ForkKind::WithContext) + } + + /// Logged here, at the one transition into the state, so a re-render never repeats it. Focus + /// leaves a thread view about to be dropped, as `handle_load_error` does; `set_server_state` + /// keeps an open fork's unsent draft as saved input, as it does for a load error. + pub(super) fn show_sub_thread_unavailable( + &mut self, + window: &mut Window, + cx: &mut Context<Self>, + ) { + if let Some(view) = self.root_thread_view() + && view + .read(cx) + .message_editor + .focus_handle(cx) + .is_focused(window) + { + self.focus_handle.focus(window, cx); + } + let session = ThreadMetadataStore::try_global(cx) + .and_then(|store| store.read(cx).entry(self.thread_id)?.session_id.clone()) + .map_or_else(|| "none".to_string(), |session| session.to_string()); + log::warn!( + "zeo_sub_thread event=unavailable thread={} session={session} code=-32002", + self.thread_id.to_key_string() + ); + self.set_server_state(ServerState::SubThreadUnavailable, cx); + } + + pub(super) fn sub_thread_unavailable_title(&self, cx: &App) -> SharedString { + ThreadMetadataStore::try_global(cx) + .and_then(|store| Some(store.read(cx).entry(self.thread_id)?.display_title())) + .unwrap_or_else(|| crate::DEFAULT_THREAD_TITLE.into()) + } + + /// The header stays above the state (R4.9), its parent title still opens the parent. + pub(super) fn render_sub_thread_unavailable(&self, cx: &App) -> AnyElement { + let inherited = self.sub_thread.inherited_messages(); + let header = + render_sub_thread_header(self.thread_id, inherited, self.workspace.clone(), cx); + v_flex() + .flex_1() + .size_full() + .children(header) + .child( + v_flex().flex_1().items_center().justify_center().child( + div() + .debug_selector(|| format!("SUB_THREAD_UNAVAILABLE-{NEVER_USED_FORK_TEXT}")) + .child(Label::new(NEVER_USED_FORK_TEXT).color(Color::Muted)), + ), + ) + .into_any_element() + } +} diff --git a/crates/agent_ui/src/conversation_view/thread_view.rs b/crates/agent_ui/src/conversation_view/thread_view.rs index ef5190c..df6578d 100644 --- a/crates/agent_ui/src/conversation_view/thread_view.rs +++ b/crates/agent_ui/src/conversation_view/thread_view.rs @@ -9777,6 +9777,7 @@ impl ThreadView { entry_ix, block.markdown().cloned(), content, + offers_slash_commands, cx, )) }), @@ -9852,7 +9853,13 @@ impl ThreadView { this.toggle_thinking_block_expansion(key, window, cx); })) .map(|header| { - self.render_message_context_menu(entry_ix, None, header.into_any(), cx) + self.render_message_context_menu( + entry_ix, + None, + header.into_any(), + false, + cx, + ) }), ) .when(is_open, |this| { @@ -9892,6 +9899,7 @@ impl ThreadView { entry_ix: usize, markdown: Option<Entity<Markdown>>, message_body: AnyElement, + offers_sub_threads: bool, cx: &Context<Self>, ) -> AnyElement { let entity = cx.entity(); @@ -9915,6 +9923,7 @@ impl ThreadView { .and_then(|markdown| markdown.context_menu_selected_text().cloned()); let selected_markdown = markdown .and_then(|markdown| markdown.context_menu_selected_markdown().cloned()); + let excerpt = selected_markdown.clone().or_else(|| selected_text.clone()); let copy_this_agent_response = ContextMenuEntry::new("Copy This Agent Response").handler({ @@ -9991,6 +10000,10 @@ impl ThreadView { }, ) }) + .map(|menu| { + let offers = offers_sub_threads && this.shows_root_session(cx); + crate::sub_thread::menu_entries(menu, &entity, excerpt, offers, cx) + }) .item(copy_this_agent_response) .separator() .item(scroll_item) @@ -14232,7 +14245,11 @@ impl ThreadView { IconName::ThreadFromSummary, "New Thread with This Command", "new thread", - AgentPanel::new_thread_with_prefilled_prompt, + |panel, agent, session, text, window, cx| { + panel.new_thread_with_prefilled_prompt( + agent, session, text, true, window, cx, + ); + }, ), ] } @@ -15009,6 +15026,7 @@ impl Render for ThreadView { let list_state = self.list_state.clone(); let conversation = v_flex() + .children(self.sub_thread_header(cx)) .when(self.resumed_without_history, |this| { this.child(Self::render_resume_notice(cx)) }) diff --git a/crates/agent_ui/src/sub_thread.rs b/crates/agent_ui/src/sub_thread.rs new file mode 100644 index 0000000..5fb89d0 --- /dev/null +++ b/crates/agent_ui/src/sub_thread.rs @@ -0,0 +1,350 @@ +//! Sub-threads opened from a selection in a thread (story 039): a clean thread that starts +//! from the quoted excerpt, or a fork of the parent session that inherits its conversation. + +mod forked_sub_thread; +mod message_menu; + +pub(crate) use message_menu::menu_entries; + +use std::sync::atomic::{AtomicBool, Ordering}; + +use acp_thread::{AgentThreadEntry, AssistantMessageChunk}; +use agent_client_protocol::v2::schema as acp_v2; +use anyhow::{Result, anyhow}; +use gpui::{App, Context, Entity, SharedString, Subscription, Task, Window}; +use util::path_list::PathList; + +use crate::conversation_view::{ConversationView, RootThreadUpdated}; +use crate::thread_metadata_store::{ThreadId, ThreadMetadataStore}; +use crate::thread_organization::{ForkKind, ThreadOrganizationStore}; +use crate::{Agent, AgentPanel}; + +/// What the "sub-thread" menu entries hand to the panel. +pub struct SubThreadRequest { + pub parent_thread_id: ThreadId, + pub parent_session_id: acp_v2::SessionId, + pub agent: Agent, + pub work_dirs: Option<PathList>, + pub kind: ForkKind, + pub excerpt: SharedString, + /// Known only when the fork is made (R4.4); `None` after a restart (R4.5). + pub inherited_messages: Option<usize>, +} + +/// The selection as a Markdown quote followed by an empty line, so the composer opens with the +/// cursor below it (R2.2, R3.2). Only the selection's end is trimmed: an interior line's +/// trailing spaces are a Markdown hard break, and leading indentation is code. +pub fn quote_excerpt(excerpt: &str) -> String { + let mut quoted = String::new(); + for line in excerpt.trim_end().lines() { + if line.is_empty() { + quoted.push('>'); + } else { + quoted.push_str("> "); + quoted.push_str(line); + } + quoted.push('\n'); + } + quoted.push('\n'); + quoted +} + +/// The user and agent messages a fork inherits (R4.4). Counted per entry, not per chunk; an +/// agent entry holding only thoughts is not a message, and neither is a tool call. +pub fn inherited_message_count(entries: &[AgentThreadEntry]) -> usize { + entries + .iter() + .filter(|entry| match entry { + // A message between sessions, such as a parent and its subagent, is part of the + // transcript the fork inherits. + AgentThreadEntry::UserMessage(_) | AgentThreadEntry::SessionMessage(_) => true, + AgentThreadEntry::AssistantMessage(message) => message + .chunks + .iter() + .any(|chunk| matches!(chunk, AssistantMessageChunk::Message { .. })), + AgentThreadEntry::ToolCall(_) + | AgentThreadEntry::Elicitation(_) + | AgentThreadEntry::ContextCompaction(_) => false, + }) + .count() +} + +/// A sub-thread's link to its parent, held by the child's view until the child has a session: +/// the organization store refuses to link a draft (story 031), and a clean sub-thread stays one +/// until its first message. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) struct PendingParentLink { + pub child: ThreadId, + pub parent: ThreadId, + pub kind: ForkKind, +} + +/// What a `ConversationView` keeps for the sub-thread it may be. +#[derive(Default)] +pub(crate) struct SubThreadViewState { + pending_parent_link: Option<PendingParentLink>, + // Known only when the fork is made (R4.4); read by the inherited band. + inherited_messages: Option<usize>, +} + +impl SubThreadViewState { + pub(crate) fn inherited_messages(&self) -> Option<usize> { + self.inherited_messages + } +} + +impl AgentPanel { + /// Opens a sub-thread of `request.parent_thread_id` with the quoted excerpt in its composer, + /// unsent, and resolves to its conversation. Takes ids and text rather than a thread view, so + /// a caller other than the message menu (a devtools sub-thread) can use it. + pub fn open_sub_thread( + &mut self, + request: SubThreadRequest, + focus: bool, + window: &mut Window, + cx: &mut Context<Self>, + ) -> Task<Result<Entity<ConversationView>>> { + match request.kind { + ForkKind::Clean => Task::ready(self.open_clean_sub_thread(request, focus, window, cx)), + ForkKind::WithContext => self.open_forked_sub_thread(request, focus, window, cx), + } + } + + fn open_clean_sub_thread( + &mut self, + request: SubThreadRequest, + focus: bool, + window: &mut Window, + cx: &mut Context<Self>, + ) -> Result<Entity<ConversationView>> { + let SubThreadRequest { + parent_thread_id, + parent_session_id, + agent, + excerpt, + .. + } = request; + let agent_id = agent.id(); + let Some(view) = self.new_thread_with_prefilled_prompt( + agent, + parent_session_id.clone(), + quote_excerpt(&excerpt), + focus, + window, + cx, + ) else { + // 0045 opens nothing in two cases. A panel with no open project is skipped without a + // word, so it is said here; a gone agent has already had 0045's toast and warning + // (R2.5), and nothing is added to them. + if !self.has_open_project(cx) { + log::warn!( + "zeo_sub_thread event=open_failed parent_session={parent_session_id} kind=clean reason=no_open_project" + ); + return Err(anyhow!("the agent panel has no open project")); + } + return Err(anyhow!("agent {agent_id} is not available")); + }; + let child = view.read(cx).thread_id; + view.update(cx, |view, _| { + view.set_pending_parent_link(PendingParentLink { + child, + parent: parent_thread_id, + kind: ForkKind::Clean, + }) + }); + log_created_once_connected( + &view, + parent_session_id, + ForkKind::Clean, + excerpt.chars().count(), + cx, + ); + Ok(view) + } +} + +/// A clean sub-thread's session exists only once its view has connected, after the open returns. +fn log_created_once_connected( + view: &Entity<ConversationView>, + parent_session: acp_v2::SessionId, + kind: ForkKind, + excerpt_chars: usize, + cx: &mut Context<AgentPanel>, +) { + fn root_session(view: &Entity<ConversationView>, cx: &App) -> Option<acp_v2::SessionId> { + let thread = view.read(cx).root_thread(cx)?; + Some(thread.read(cx).session_id().clone()) + } + if let Some(session) = root_session(view, cx) { + log_sub_thread_created(&parent_session, &session, kind, excerpt_chars); + return; + } + let mut parent_session = Some(parent_session); + cx.subscribe(view, move |_, view, _: &RootThreadUpdated, cx| { + if parent_session.is_some() + && let Some(session) = root_session(&view, cx) + && let Some(parent_session) = parent_session.take() + { + log_sub_thread_created(&parent_session, &session, kind, excerpt_chars); + } + }) + .detach(); +} + +/// R7.1. Ids, the kind and a length only: the excerpt and the titles are the user's text. +/// `zeo_sub_thread` is in the message, not the log target, so every sink (and 0048's test +/// capture, which reads only the message) sees it. +pub(crate) fn log_sub_thread_created( + parent_session: &acp_v2::SessionId, + session: &acp_v2::SessionId, + kind: ForkKind, + excerpt_chars: usize, +) { + log::info!( + "zeo_sub_thread event=created parent_session={parent_session} session={session} kind={} excerpt_chars={excerpt_chars}", + fork_kind_label(kind) + ); +} + +// 0048 keeps its own spelling private; this is the same one, for log lines. +fn fork_kind_label(kind: ForkKind) -> &'static str { + match kind { + ForkKind::WithContext => "with_context", + ForkKind::Clean => "clean", + } +} + +/// Writes the link once; a refusal leaves the sub-thread open and usable (R7.2). The caller has +/// already dropped the pending link, so a refused link is never retried: `set_parent` would +/// refuse every later attempt the same way, or with "already has a parent". +pub(crate) fn record_parent_link(child: ThreadId, parent: ThreadId, kind: ForkKind, cx: &mut App) { + static STORE_ABSENT_WARNED: AtomicBool = AtomicBool::new(false); + let Some(store) = ThreadOrganizationStore::try_global(cx) else { + if !STORE_ABSENT_WARNED.swap(true, Ordering::Relaxed) { + log::warn!( + "zeo_sub_thread event=link_failed reason=no_organization_store: sub-thread links are not recorded" + ); + } + return; + }; + if let Err(error) = store.update(cx, |store, cx| store.set_parent(child, parent, kind, cx)) { + log::warn!( + "zeo_sub_thread event=link_failed child={} parent={} kind={}: {error}", + child.to_key_string(), + parent.to_key_string(), + fork_kind_label(kind) + ); + } +} + +/// R2.6: a clean sub-thread whose composer was emptied is reused as the panel's next new thread, +/// which is no longer the sub-thread. Dropped here, at the reuse, rather than when the view moves +/// into the draft slot: a first send also empties the composer, and the view passes through that +/// slot until the sent message lands. +pub(crate) fn drop_link_of_reused_draft( + draft: &Entity<ConversationView>, + has_editor_content: bool, + cx: &mut App, +) { + if !has_editor_content { + draft.update(cx, |view, _| view.take_pending_parent_link()); + } +} + +/// The observations a `ConversationView` keeps for its sub-thread state: the pending link waits +/// on the metadata store, and the header (R4) changes with both stores. +pub(crate) fn observe_sub_thread_stores(cx: &mut Context<ConversationView>) -> Vec<Subscription> { + let mut subscriptions = Vec::new(); + if let Some(metadata) = ThreadMetadataStore::try_global(cx) { + subscriptions.push(cx.observe(&metadata, |view, _, cx| view.sub_thread_stores_changed(cx))); + } + if let Some(organization) = ThreadOrganizationStore::try_global(cx) { + subscriptions.push(cx.observe(&organization, |view, _, cx| { + view.sub_thread_stores_changed(cx) + })); + } + subscriptions +} + +/// The metadata store withholds a draft's session id, and the organization store admits only a +/// thread that has one. +fn thread_has_session(thread_id: ThreadId, cx: &App) -> bool { + ThreadMetadataStore::try_global(cx).is_some_and(|store| { + store + .read(cx) + .entry(thread_id) + .is_some_and(|entry| !entry.is_draft()) + }) +} + +impl ConversationView { + pub(crate) fn set_pending_parent_link(&mut self, link: PendingParentLink) { + self.sub_thread.pending_parent_link = Some(link); + } + + pub(crate) fn take_pending_parent_link(&mut self) -> Option<PendingParentLink> { + self.sub_thread.pending_parent_link.take() + } + + fn sub_thread_stores_changed(&mut self, cx: &mut Context<Self>) { + if self + .sub_thread + .pending_parent_link + .is_some_and(|link| thread_has_session(link.child, cx)) + && let Some(link) = self.take_pending_parent_link() + { + // Deferred: a store write reached from a metadata-store change re-enters it (story + // 031), and the organization store reads the metadata store to admit the link. + cx.defer(move |cx| record_parent_link(link.child, link.parent, link.kind, cx)); + } + let linked = ThreadOrganizationStore::try_global(cx) + .is_some_and(|store| store.read(cx).parent(self.thread_id).is_some()); + if !linked { + return; + } + match self.root_thread_view() { + Some(thread_view) => thread_view.update(cx, |_, cx| cx.notify()), + // The unavailable state (R4.9) draws the header itself, with no thread view. + None => cx.notify(), + } + } +} + +#[cfg(test)] +#[path = "sub_thread_tests/pure.rs"] +mod pure; + +#[cfg(test)] +#[path = "sub_thread_tests/support.rs"] +mod test_support; + +#[cfg(test)] +#[path = "sub_thread_tests/clean.rs"] +mod clean; + +#[cfg(test)] +#[path = "sub_thread_tests/fork.rs"] +mod fork; + +#[cfg(test)] +#[path = "sub_thread_tests/fork_abandoned.rs"] +mod fork_abandoned; + +// A frozen assertion clones a session id on its last use; the lint is silenced here rather than +// edited there. +#[cfg(test)] +#[allow(clippy::redundant_clone)] +#[path = "sub_thread_tests/menu.rs"] +mod menu; + +#[cfg(test)] +#[path = "sub_thread_tests/menu_subagent.rs"] +mod menu_subagent; + +#[cfg(test)] +#[path = "sub_thread_tests/header.rs"] +mod header; + +#[cfg(test)] +#[path = "sub_thread_tests/reopen.rs"] +mod reopen; diff --git a/crates/agent_ui/src/sub_thread/forked_sub_thread.rs b/crates/agent_ui/src/sub_thread/forked_sub_thread.rs new file mode 100644 index 0000000..d43d3cc --- /dev/null +++ b/crates/agent_ui/src/sub_thread/forked_sub_thread.rs @@ -0,0 +1,221 @@ +//! The "with context" sub-thread (story 039, R3): fork the parent session over ACP and open the +//! fork's registered thread, or fail the one way R3.6 describes. + +use std::rc::Rc; + +use acp_thread::{AcpThread, AgentConnection, ProtocolErrorInfo}; +use agent_client_protocol::v2::schema as acp_v2; +use anyhow::{Result, anyhow}; +use gpui::{AsyncApp, Context, Entity, Task, Window}; +use util::ResultExt as _; +use util::path_list::PathList; +use workspace::Toast; +use workspace::notifications::NotificationId; + +use super::{PendingParentLink, SubThreadRequest, log_sub_thread_created, quote_excerpt}; +use crate::conversation_view::{ConversationView, RootThreadUpdated}; +use crate::thread_organization::ForkKind; +use crate::{AgentInitialContent, AgentPanel, AgentThreadSource}; + +impl AgentPanel { + /// R3: forks the parent session and opens the fork's registered thread. Every failure on the + /// way, whatever its cause, is the one of R3.6: a toast, a warning, nothing opened or linked. + pub(super) fn open_forked_sub_thread( + &mut self, + request: SubThreadRequest, + focus: bool, + window: &mut Window, + cx: &mut Context<Self>, + ) -> Task<Result<Entity<ConversationView>>> { + // R3.8: without directories the fork request could not say where the session works. + let Some(work_dirs) = request.work_dirs.clone() else { + let error = anyhow!("the parent thread has no working directories"); + return Task::ready(Err(self.sub_thread_fork_failed( + &request.parent_session_id, + error, + cx, + ))); + }; + let store = self.connection_store().read(cx); + let connecting = store + .entry(&request.agent) + .map(|entry| entry.read(cx).wait_for_connection()); + let project = store.project().clone(); + let agent_id = request.agent.id(); + let parent_session = request.parent_session_id.clone(); + cx.spawn_in(window, async move |this, cx| { + let forked: Result<(Rc<dyn AgentConnection>, Entity<AcpThread>)> = async { + let connecting = + connecting.ok_or_else(|| anyhow!("agent {agent_id} has no connection"))?; + let connection = connecting.await.map_err(anyhow::Error::new)?.connection; + let thread = cx + .update(|_, cx| { + connection.clone().fork_session( + &parent_session, + project, + work_dirs.clone(), + cx, + ) + })? + .await?; + Ok((connection, thread)) + } + .await; + let opened = forked.and_then(|(connection, thread)| { + let fork = thread.clone(); + this.update_in(cx, |panel, window, cx| { + panel.open_forked_thread(request, thread, work_dirs, focus, window, cx) + }) + // The fork exists in the agent and is registered, so v1's abandoned-fork close + // will not fire: nothing else would ever close it. + .inspect_err(|_| close_abandoned_fork(connection, &fork, cx)) + }); + opened.inspect_err(|error| { + // A released panel takes its toast with it; the warning below still says why. + if this + .update(cx, |panel, cx| panel.show_sub_thread_failed_toast(cx)) + .is_err() + { + log::debug!("zeo_sub_thread event=toast_skipped reason=panel_released"); + } + warn_sub_thread_fork_failed(&parent_session, error); + }) + }) + } + + /// The fork exists in the agent: open its registered thread rather than load it (design + /// *Decision*), with the quote unsent, and leave the link to the view's first metadata save. + fn open_forked_thread( + &mut self, + request: SubThreadRequest, + thread: Entity<AcpThread>, + work_dirs: PathList, + focus: bool, + window: &mut Window, + cx: &mut Context<Self>, + ) -> Entity<ConversationView> { + let fork_session = thread.read(cx).session_id().clone(); + let quote = + acp_v2::ContentBlock::Text(acp_v2::TextContent::new(quote_excerpt(&request.excerpt))); + let opened = self.create_agent_thread_with_server_for_external_session( + request.agent, + None, + fork_session.clone(), + Some(work_dirs), + None, + Some(AgentInitialContent::ContentBlock { + blocks: vec![quote], + auto_submit: false, + }), + AgentThreadSource::AgentPanel, + window, + cx, + ); + let view = self.show_agent_thread(opened, focus, window, cx); + let child = view.read(cx).thread_id; + view.update(cx, |view, _| { + view.set_pending_parent_link(PendingParentLink { + child, + parent: request.parent_thread_id, + kind: ForkKind::WithContext, + }); + view.sub_thread.inherited_messages = request.inherited_messages; + }); + hold_fork_until_attached(&view, thread, cx); + log_sub_thread_created( + &request.parent_session_id, + &fork_session, + ForkKind::WithContext, + request.excerpt.chars().count(), + ); + view + } + + fn sub_thread_fork_failed( + &self, + parent_session: &acp_v2::SessionId, + error: anyhow::Error, + cx: &mut Context<Self>, + ) -> anyhow::Error { + self.show_sub_thread_failed_toast(cx); + warn_sub_thread_fork_failed(parent_session, &error); + error + } + + fn show_sub_thread_failed_toast(&self, cx: &mut Context<Self>) { + struct SubThreadFailedToast; + self.panel_workspace() + .update(cx, |workspace, cx| { + workspace.show_toast( + Toast::new( + NotificationId::unique::<SubThreadFailedToast>(), + "Couldn't create the sub-thread", + ), + cx, + ); + }) + .log_err(); + } +} + +/// R3.6. `code` is the agent's JSON-RPC error code, `none` when the failure is not the agent's +/// (no work dirs, no connection, a released panel). The error text is the agent's or Zeo's, +/// never the user's. +fn warn_sub_thread_fork_failed(parent_session: &acp_v2::SessionId, error: &anyhow::Error) { + let code = ProtocolErrorInfo::from_error(error) + .map_or_else(|| "none".to_string(), |info| info.code.to_string()); + log::warn!( + "zeo_sub_thread event=fork_failed parent_session={parent_session} code={code} error={:?}", + format!("{error:#}") + ); +} + +/// Design *Error Handling Strategy*: a fork whose panel or window went away before it could be +/// shown is closed in the agent when the agent supports close; otherwise it stays open there until +/// the agent exits, which is worth a warning. Detached: the failure has already been decided. +fn close_abandoned_fork( + connection: Rc<dyn AgentConnection>, + fork: &Entity<AcpThread>, + cx: &AsyncApp, +) { + cx.update(|cx| { + let session = fork.read(cx).session_id().clone(); + if !connection.supports_close_session() { + log::warn!("zeo_sub_thread event=fork_abandoned session={session} close=unsupported"); + return; + } + let closing = connection.close_session(&session, cx); + cx.spawn(async move |_| match closing.await { + Ok(()) => { + log::debug!("zeo_sub_thread event=fork_abandoned session={session} close=ok") + } + Err(error) => log::warn!( + "zeo_sub_thread event=fork_abandoned session={session} close=failed error={:?}", + format!("{error:#}") + ), + }) + .detach(); + }); +} + +/// `fork_session` registers its thread weakly, and the view loads it only after the open returns: +/// without this strong reference the load would find the registration dead and send a real +/// `session/load` (design *Implementation Gotchas*). Dropped once the view has attached it. +fn hold_fork_until_attached( + view: &Entity<ConversationView>, + thread: Entity<AcpThread>, + cx: &mut Context<AgentPanel>, +) { + let fork_session = thread.read(cx).session_id().clone(); + let mut held = Some(thread); + cx.subscribe(view, move |_, view, _: &RootThreadUpdated, cx| { + let attached = view + .read(cx) + .root_thread(cx) + .is_some_and(|root| *root.read(cx).session_id() == fork_session); + if attached { + held.take(); + } + }) + .detach(); +} diff --git a/crates/agent_ui/src/sub_thread/message_menu.rs b/crates/agent_ui/src/sub_thread/message_menu.rs new file mode 100644 index 0000000..5966c0c --- /dev/null +++ b/crates/agent_ui/src/sub_thread/message_menu.rs @@ -0,0 +1,133 @@ +//! The two sub-thread entries in an agent message's context menu (story 039, R1). + +use acp_thread::ThreadStatus; +use gpui::{App, Entity, SharedString, Window}; +use ui::{ContextMenu, ContextMenuEntry, DocumentationSide, Label, prelude::*}; + +use super::{SubThreadRequest, inherited_message_count}; +use crate::AgentPanel; +use crate::conversation_view::ThreadView; +use crate::thread_organization::ForkKind; + +/// R1.6's tooltip, shown beside the disabled entry the way upstream's menus explain theirs. +const WAIT_FOR_TURN: &str = "Wait for the turn to end"; + +/// Adds "Clean sub-thread" and, when the agent can fork a session and open the fork, "Sub-thread +/// with context" after a separator. `offers_sub_threads` is the caller's word that the menu belongs +/// to an agent's message (not a thinking block, R1.5); `excerpt` is the selection, as Markdown when +/// it has some. Anything else leaves the menu as it was (R1.4, R1.7). +pub(crate) fn menu_entries( + menu: ContextMenu, + thread_view: &Entity<ThreadView>, + excerpt: Option<SharedString>, + offers_sub_threads: bool, + cx: &App, +) -> ContextMenu { + let Some(excerpt) = excerpt.filter(|excerpt| !excerpt.trim().is_empty()) else { + return menu; + }; + if !offers_sub_threads { + return menu; + } + let thread = thread_view.read(cx).thread.read(cx); + let connection = thread.connection(); + // R1.8: the panel opens a fork through the load-or-resume path, so without either the fork + // would exist in the agent and never be shown. + let offers_fork = connection.supports_fork_session() + && (connection.supports_load_session() || connection.supports_resume_session()); + let turn_running = thread.status() != ThreadStatus::Idle; + + let entry = |label: &'static str, kind: ForkKind| { + let thread_view = thread_view.clone(); + let excerpt = excerpt.clone(); + ContextMenuEntry::new(label).handler(move |window, cx| { + open_from_menu(&thread_view, kind, excerpt.clone(), window, cx) + }) + }; + menu.separator() + .item(entry("Clean sub-thread", ForkKind::Clean)) + .when(offers_fork, |menu| { + let fork = entry("Sub-thread with context", ForkKind::WithContext); + menu.item(if turn_running { + fork.disabled(true) + .documentation_aside(DocumentationSide::Left, |_| { + Label::new(WAIT_FOR_TURN).into_any_element() + }) + } else { + fork + }) + }) +} + +impl ThreadView { + /// R1.9: false in a subagent's view, where the session is the subagent's while the panel's + /// thread id is the root conversation's, so a fork would inherit one and link the other. + /// `parent_session_id` alone misses it: only the native agent sets it, and an ACP adapter's + /// subagent is loaded with `None` (`agent_servers/src/acp/v1.rs`), so the root session decides. + pub(crate) fn shows_root_session(&self, cx: &App) -> bool { + self.parent_session_id.is_none() + && self.server_view.upgrade().is_some_and(|conversation| { + conversation.read(cx).root_session_id.as_ref() == Some(&self.session_id) + }) + } +} + +/// Builds the request from the thread as it is at the click, and opens it focused. +fn open_from_menu( + thread_view: &Entity<ThreadView>, + kind: ForkKind, + excerpt: SharedString, + window: &mut Window, + cx: &mut App, +) { + let view = thread_view.read(cx); + let thread = view.thread.read(cx); + let parent_session_id = thread.session_id().clone(); + let kind_label = super::fork_kind_label(kind); + // The panel's key for the conversation, not the connection's own agent id: the connection + // store, which the fork goes through, is keyed by it. + let Some(agent) = view + .server_view + .upgrade() + .map(|conversation| conversation.read(cx).agent_key().clone()) + else { + log::warn!( + "zeo_sub_thread event=open_failed parent_session={parent_session_id} kind={kind_label} reason=no_conversation" + ); + return; + }; + let request = SubThreadRequest { + parent_thread_id: view.root_thread_id, + parent_session_id: parent_session_id.clone(), + agent, + work_dirs: thread.work_dirs().cloned(), + kind, + excerpt, + inherited_messages: match kind { + ForkKind::WithContext => Some(inherited_message_count(thread.entries())), + ForkKind::Clean => None, + }, + }; + let Some(panel) = view + .workspace + .upgrade() + .and_then(|workspace| workspace.read(cx).panel::<AgentPanel>(cx)) + else { + log::warn!( + "zeo_sub_thread event=open_failed parent_session={parent_session_id} kind={kind_label} reason=no_agent_panel" + ); + return; + }; + let opening = panel.update(cx, |panel, cx| { + panel.open_sub_thread(request, true, window, cx) + }); + // Detached, not dropped: dropping the task would cancel a fork in flight without a word. + cx.spawn(async move |_| { + if let Err(error) = opening.await { + log::warn!( + "zeo_sub_thread event=open_failed parent_session={parent_session_id} kind={kind_label} source=menu: {error:#}" + ); + } + }) + .detach(); +} diff --git a/crates/agent_ui/src/sub_thread_tests/clean.rs b/crates/agent_ui/src/sub_thread_tests/clean.rs new file mode 100644 index 0000000..67f6725 --- /dev/null +++ b/crates/agent_ui/src/sub_thread_tests/clean.rs @@ -0,0 +1,385 @@ +// Story 039, task 4.2 — "Clean sub-thread": a new prefilled thread whose parent link is recorded on +// its first send (R2.1–R2.6, R7.1, R7.2; Q4, Q13). Wiring and seams: `support.rs`. Filter: +// `cargo test -p agent_ui sub_thread::clean`. +// +// Contract: `open_sub_thread(SubThreadRequest { kind: ForkKind::Clean, .. }, focus, ..)` opens the +// thread through `0045`'s `new_thread_with_prefilled_prompt`, which now returns +// `Option<Entity<ConversationView>>` (`None` for an unavailable agent, and `open_sub_thread` then +// resolves to `Err`); the link waits for the first send because +// `ThreadOrganizationStore::set_parent` refuses a draft (story 031). +// +// Hostile halves first (identity of the pending link): +// - wrong collapse: a pending link belongs to ITS draft — another draft getting its first +// message must not take it, and a discarded draft's link must not land anywhere; +// - wrong split: a second message in the same sub-thread records nothing more (no second +// `set_parent`, so no "already has a parent" refusal in the log). + +use gpui::{Focusable as _, TestAppContext}; +use project::AgentId; + +use super::test_support::*; +use crate::Agent; +use crate::thread_organization::ForkKind; +use acp_thread::StubAgentConnection; + +const EXCERPT: &str = "Ship **v2** first\n\nthen tag"; +const QUOTED: &str = "> Ship **v2** first\n>\n> then tag\n\n"; + +fn clean(parent: &Parent, excerpt: &str) -> super::SubThreadRequest { + request(parent, ForkKind::Clean, excerpt, None) +} + +// Hostile half (wrong collapse) — two pending clean sub-threads of one parent: the first message +// of B links B only; A stays unlinked until its own first message. +#[gpui::test] +async fn clean_hostile_a_pending_link_belongs_to_its_own_draft(cx: &mut TestAppContext) { + let (h, mut cx) = setup(StubAgentConnection::new(), cx).await; + let parent = open_parent(&h, vec![agent_says("Tag, then ship.")], &mut cx); + open_sub_thread(&h, clean(&parent, "Tag"), &mut cx); + let a = active_id(&h, &cx); + open_sub_thread(&h, clean(&parent, "ship"), &mut cx); + let b = active_id(&h, &cx); + assert_ne!(a, b, "fixture: two sub-threads"); + + send_text(&h, "Why ship first?", &mut cx); + + assert_eq!(parent_link(b, &mut cx), Some((parent.id, ForkKind::Clean))); + assert_eq!(parent_link(a, &mut cx), None, "A is still a draft"); + assert_eq!(children(parent.id, &mut cx), vec![b]); + + activate(&h, a, &mut cx); + send_text(&h, "Why tag?", &mut cx); + assert_eq!(parent_link(a, &mut cx), Some((parent.id, ForkKind::Clean))); +} + +// Hostile half (wrong collapse) — R2.4: a draft discarded before its first message records +// nothing, and the next thread to get a first message does not inherit its pending link. +#[gpui::test] +async fn clean_hostile_a_discarded_draft_records_nothing(cx: &mut TestAppContext) { + let (h, mut cx) = setup(StubAgentConnection::new(), cx).await; + let parent = open_parent(&h, vec![agent_says("Tag, then ship.")], &mut cx); + open_sub_thread(&h, clean(&parent, EXCERPT), &mut cx); + let child = active_id(&h, &cx); + + remove(&h, child, &mut cx); + let next = active_id(&h, &cx); + assert_ne!(next, child, "fixture: the discarded draft is gone"); + send_text(&h, "Something else entirely.", &mut cx); + + assert_eq!(parent_link(child, &mut cx), None); + assert_eq!( + parent_link(next, &mut cx), + None, + "the pending link died with its draft" + ); + assert!( + children(parent.id, &mut cx).is_empty(), + "R2.4: no link stored" + ); +} + +// Hostile half (wrong split) — a second message records nothing more: one link, and no refused +// second `set_parent` in the log. +#[gpui::test] +async fn clean_hostile_a_second_message_records_nothing_more(cx: &mut TestAppContext) { + let (h, mut cx) = setup(StubAgentConnection::new(), cx).await; + let parent = open_parent(&h, vec![agent_says("Tag, then ship.")], &mut cx); + open_sub_thread(&h, clean(&parent, EXCERPT), &mut cx); + let child = active_id(&h, &cx); + capture_logs(); + + send_text(&h, "First.", &mut cx); + send_text(&h, "Second.", &mut cx); + + assert_eq!( + parent_link(child, &mut cx), + Some((parent.id, ForkKind::Clean)) + ); + assert_eq!(children(parent.id, &mut cx), vec![child]); + assert!( + all_lines_lowercased() + .iter() + .all(|line| !line.contains("already has a parent")), + "set_parent is called once per sub-thread" + ); +} + +// R2.1, R2.2 — a new thread with the parent's agent, in the same project, focused, holding the +// quoted excerpt unsent; the parent is unchanged and kept; no link before the first send (R2.3). +#[gpui::test] +async fn clean_opens_a_focused_thread_with_the_quote_unsent(cx: &mut TestAppContext) { + let (h, mut cx) = setup(StubAgentConnection::new(), cx).await; + let parent = open_parent( + &h, + vec![agent_says("Ship **v2** first, then tag.")], + &mut cx, + ); + let parent_entries = entry_count(&parent.thread, &cx); + let parent_dirs = parent.thread.read_with(&cx, |t, _| t.work_dirs().cloned()); + let toasts_before = toasts(&h, &cx); + + open_sub_thread(&h, clean(&parent, EXCERPT), &mut cx); + + let child = active_id(&h, &cx); + assert_ne!(child, parent.id, "R2.1: a new thread"); + let child_thread = active_thread(&h, &cx); + assert_ne!( + child_thread.read_with(&cx, |t, _| t.session_id().clone()), + parent.session + ); + assert_eq!( + active_agent_id(&h, &cx), + AgentId::new("stub"), + "R2.1: the parent's agent" + ); + assert_eq!( + child_thread.read_with(&cx, |t, _| t.work_dirs().cloned()), + parent_dirs, + "R2.1: the same project" + ); + assert!(composer_focused(&h, &mut cx), "R2.1: focused"); + assert_eq!( + composer(&h, child, &cx).as_deref(), + Some(QUOTED), + "R2.2: the quote" + ); + assert_eq!(entry_count(&child_thread, &cx), 0, "R2.2: nothing sent"); + assert_eq!( + entry_count(&parent.thread, &cx), + parent_entries, + "R2.1: parent unchanged" + ); + assert!( + h.panel + .read_with(&cx, |p, _| p.retained_threads().contains_key(&parent.id)) + ); + assert_eq!(toasts(&h, &cx), toasts_before, "no toast on success"); + assert_eq!( + parent_link(child, &mut cx), + None, + "R2.3: no link before the first send" + ); +} + +// R2.3 — the first message records the link, with fork kind `clean`. +#[gpui::test] +async fn clean_first_message_records_the_clean_link(cx: &mut TestAppContext) { + let (h, mut cx) = setup(StubAgentConnection::new(), cx).await; + let parent = open_parent(&h, vec![agent_says("Tag, then ship.")], &mut cx); + open_sub_thread(&h, clean(&parent, EXCERPT), &mut cx); + let child = active_id(&h, &cx); + + send_text(&h, "Why first?", &mut cx); + + assert_eq!( + parent_link(child, &mut cx), + Some((parent.id, ForkKind::Clean)) + ); + assert_eq!( + parent_link(parent.id, &mut cx), + None, + "the parent gets no parent" + ); +} + +// R2.5 — an agent that is no longer available: nothing opens, `0045`'s toast, no link. +#[gpui::test] +async fn clean_for_an_unavailable_agent_opens_nothing(cx: &mut TestAppContext) { + let (h, mut cx) = setup(StubAgentConnection::new(), cx).await; + let parent = open_parent(&h, vec![agent_says("Tag, then ship.")], &mut cx); + let retained_before = retained(&h, &cx); + let toasts_before = toasts(&h, &cx); + let mut gone = clean(&parent, EXCERPT); + gone.agent = Agent::Custom { + id: AgentId::new("retired-agent"), + }; + + let result = try_open_sub_thread(&h, gone, true, &mut cx); + + assert!(result.is_err(), "the open resolves to an error"); + assert_eq!(active_id(&h, &cx), parent.id, "nothing opened"); + assert_eq!(retained(&h, &cx), retained_before); + assert_eq!(toasts(&h, &cx), toasts_before + 1, "one toast"); + assert!(children(parent.id, &mut cx).is_empty()); +} + +// `new_thread_with_prefilled_prompt(agent, source_session, text, focus, window, cx)` answers with +// the conversation it opened — the one now shown — +// and with `None`, opening nothing, for an agent that is gone (0045's toast). +#[gpui::test] +async fn clean_prefilled_open_returns_the_opened_conversation(cx: &mut TestAppContext) { + let (h, mut cx) = setup(StubAgentConnection::new(), cx).await; + let parent = open_parent(&h, vec![agent_says("Tag, then ship.")], &mut cx); + let toasts_before = toasts(&h, &cx); + + let gone = h.panel.update_in(&mut cx, |panel, window, cx| { + panel.new_thread_with_prefilled_prompt( + Agent::Custom { + id: AgentId::new("retired-agent"), + }, + parent.session.clone(), + QUOTED.to_string(), + true, + window, + cx, + ) + }); + settle(&mut cx); + assert!(gone.is_none(), "an unavailable agent opens nothing"); + assert_eq!(active_id(&h, &cx), parent.id); + assert_eq!(toasts(&h, &cx), toasts_before + 1); + + let opened = h.panel.update_in(&mut cx, |panel, window, cx| { + panel.new_thread_with_prefilled_prompt( + Agent::Stub, + parent.session.clone(), + QUOTED.to_string(), + true, + window, + cx, + ) + }); + settle(&mut cx); + let opened = opened.expect("an available agent opens a conversation"); + assert_eq!( + h.panel + .read_with(&cx, |panel, _| panel.active_conversation_view().cloned()), + Some(opened), + "the returned conversation is the active one" + ); + assert_eq!( + composer(&h, active_id(&h, &cx), &cx).as_deref(), + Some(QUOTED) + ); +} + +// Hostile half (wrong collapse) — R2.6: a clean sub-thread emptied before its first message +// becomes the panel's reusable empty draft; the next New Thread reuses that very view, and its +// first message — unrelated to the parent — records no link. +#[gpui::test] +async fn clean_hostile_an_emptied_draft_reused_by_new_thread_records_nothing( + cx: &mut TestAppContext, +) { + let (h, mut cx) = setup(StubAgentConnection::new(), cx).await; + let parent = open_parent(&h, vec![agent_says("Tag, then ship.")], &mut cx); + open_sub_thread(&h, clean(&parent, EXCERPT), &mut cx); + let child = active_id(&h, &cx); + + type_text(&h, "", &mut cx); + settle(&mut cx); + assert_eq!( + h.panel + .read_with(&cx, |panel, cx| panel.ephemeral_draft_thread_id(cx)), + Some(child), + "precondition: the emptied view is the panel's reusable draft" + ); + h.panel.update_in(&mut cx, |panel, window, cx| { + panel.new_thread(&crate::NewThread, window, cx) + }); + settle(&mut cx); + assert_eq!( + active_id(&h, &cx), + child, + "precondition: New Thread reused the view" + ); + send_text(&h, "Something unrelated.", &mut cx); + + assert_eq!( + parent_link(child, &mut cx), + None, + "R2.6: the pending link was dropped" + ); + assert!(children(parent.id, &mut cx).is_empty()); +} + +// `focus: false` reaches 0045: the clean sub-thread opens, quoted, without taking the focus. +#[gpui::test] +async fn clean_unfocused_open_does_not_take_the_focus(cx: &mut TestAppContext) { + let (h, mut cx) = setup(StubAgentConnection::new(), cx).await; + let parent = open_parent(&h, vec![agent_says("Tag, then ship.")], &mut cx); + + let view = try_open_sub_thread(&h, clean(&parent, EXCERPT), false, &mut cx) + .expect("the clean sub-thread opens"); + + let thread_view = view + .read_with(&cx, |view, _| view.active_thread().cloned()) + .expect("its thread view"); + let editor = thread_view.read_with(&cx, |view, _| view.message_editor.clone()); + assert!( + !cx.update(|window, cx| editor.focus_handle(cx).contains_focused(window, cx)), + "its composer did not take the focus" + ); + assert_eq!(editor.read_with(&cx, |editor, cx| editor.text(cx)), QUOTED); +} + +// R7.2 — the store refuses the link (the parent was deleted before the first message): the +// sub-thread stays open with its message, and a warning names both thread ids and the refusal. +#[gpui::test] +async fn clean_refused_link_keeps_the_thread_and_warns_with_both_ids(cx: &mut TestAppContext) { + let (h, mut cx) = setup(StubAgentConnection::new(), cx).await; + let parent = open_parent(&h, vec![agent_says("Tag, then ship.")], &mut cx); + open_sub_thread(&h, clean(&parent, EXCERPT), &mut cx); + let child = active_id(&h, &cx); + remove(&h, parent.id, &mut cx); + assert_eq!( + active_id(&h, &cx), + child, + "fixture: the sub-thread is still active" + ); + capture_logs(); + + send_text(&h, "Still here?", &mut cx); + + assert_eq!(active_id(&h, &cx), child, "the sub-thread stays open"); + assert_eq!( + user_messages(&active_thread(&h, &cx), &cx), + 1, + "its message was sent" + ); + assert_eq!(parent_link(child, &mut cx), None, "nothing stored"); + let child_key = child.to_key_string().to_lowercase(); + let parent_key = parent.id.to_key_string().to_lowercase(); + assert!( + all_lines_lowercased() + .iter() + .any(|line| line.contains(&child_key) + && line.contains(&parent_key) + && line.contains("unknown")), + "a warning names both ids and the store's refusal: {:?}", + all_lines_lowercased() + ); +} + +// R7.1 — one `zeo_sub_thread event=created` line with both sessions, `kind=clean` and the +// excerpt's length in CHARACTERS; never the excerpt's text or a title. The excerpt is 12 +// characters in 16 bytes, so a byte count is caught. +#[gpui::test] +async fn clean_logs_kind_and_length_never_text(cx: &mut TestAppContext) { + let (h, mut cx) = setup(StubAgentConnection::new(), cx).await; + let parent = open_parent(&h, vec![agent_says("naïve café ☕")], &mut cx); + capture_logs(); + let excerpt = "naïve café ☕"; + assert_eq!( + (excerpt.chars().count(), excerpt.len()), + (12, 16), + "fixture" + ); + + open_sub_thread(&h, clean(&parent, excerpt), &mut cx); + let child_session = active_thread(&h, &cx).read_with(&cx, |t, _| t.session_id().0.to_string()); + send_text(&h, "Why?", &mut cx); + + // A clean sub-thread's session is accepted by the store only from its first message, so the + // line may come at open or at link time — once. + assert_one_created_line(&parent.session.0, &child_session, "clean", 12); + for line in all_lines_lowercased() { + assert!( + !line.contains("naïve café"), + "the excerpt reached a log: {line}" + ); + assert!( + !line.contains(&PARENT_TITLE.to_lowercase()), + "a title reached a log: {line}" + ); + } +} diff --git a/crates/agent_ui/src/sub_thread_tests/fork.rs b/crates/agent_ui/src/sub_thread_tests/fork.rs new file mode 100644 index 0000000..9099337 --- /dev/null +++ b/crates/agent_ui/src/sub_thread_tests/fork.rs @@ -0,0 +1,329 @@ +// Story 039, task 4.3 — "Sub-thread with context": fork the parent session and open the fork, +// empty, with the quote unsent and the link recorded at once (R3.1–R3.7, R7.1; Q4, Q13). Wiring +// and seams: `support.rs`. Filter: `cargo test -p agent_ui sub_thread::fork`. +// +// Contract: `open_sub_thread(SubThreadRequest { kind: ForkKind::WithContext, .. }, focus, ..)` +// fails at once without work dirs (R3.8); otherwise calls the parent connection's +// `fork_session`, whose thread is marked `forked_from` (task 3.3) and so is saved with its session +// id while still empty; opens that registered session (no `session/load`, design.md Decision), +// keeping the forked entity alive until the view's first `RootThreadUpdated`; and records the +// link through `PendingParentLink { kind: WithContext }` before anything is sent. +// +// "No load request" over the stub: the stub builds a thread only on `new_session`, `load_session` +// or `fork_session`; task 3.1 pins that a forked id reopened is the registered thread. So the +// observable here is that the view shows the fork EMPTY, on the fork's session, with exactly one +// fork asked of the connection. The ACP-level "no session/load on the wire" is task 3.2's +// `fork_registered_thread_reopens_without_a_load_request`. +// +// Hostile halves first (identity of a fork): +// - wrong collapse: a message in the fork never lands in the parent, and a second fork of the +// same parent is a second thread with its own link, not the first one re-shown; +// - wrong split: the second fork leaves the first fork's link and composer exactly as they were. + +use gpui::{Entity, Focusable as _, TestAppContext, VisualTestContext}; +use project::AgentId; + +use super::test_support::*; +use crate::ConversationView; +use crate::thread_metadata_store::ThreadMetadataStore; +use crate::thread_organization::ForkKind; +use acp_thread::StubAgentConnection; + +const EXCERPT: &str = "**Quincunx**"; +const QUOTED: &str = "> **Quincunx**\n\n"; + +fn forking() -> StubAgentConnection { + StubAgentConnection::new() + .with_supports_load_session(true) + .with_fork_support() +} + +fn with_context(parent: &Parent, excerpt: &str) -> super::SubThreadRequest { + request(parent, ForkKind::WithContext, excerpt, Some(2)) +} + +fn session_of(h: &Harness, cx: &gpui::VisualTestContext) -> String { + active_thread(h, cx).read_with(cx, |t, _| t.session_id().0.to_string()) +} + +// Hostile half (wrong collapse) — R3.4: a message sent in the fork reaches the fork's session and +// never the parent's. +#[gpui::test] +async fn fork_hostile_a_message_in_the_fork_never_reaches_the_parent(cx: &mut TestAppContext) { + let (h, mut cx) = setup(forking(), cx).await; + let parent = open_parent(&h, vec![agent_says(EXCERPT)], &mut cx); + open_sub_thread(&h, with_context(&parent, EXCERPT), &mut cx); + let fork = active_thread(&h, &cx); + assert_eq!(user_messages(&parent.thread, &cx), 1, "fixture"); + + h.connection + .set_next_prompt_updates(vec![agent_says("It is the token.")]); + send_text(&h, "Which token?", &mut cx); + + assert_eq!(user_messages(&fork, &cx), 1, "the fork got the message"); + assert_eq!(user_messages(&parent.thread, &cx), 1, "the parent did not"); +} + +// Hostile half (wrong collapse, then wrong split) — R3.7: a second fork is a second thread with +// its own link, and the first fork keeps its link and its composer. +#[gpui::test] +async fn fork_hostile_a_second_fork_is_a_second_thread(cx: &mut TestAppContext) { + let (h, mut cx) = setup(forking(), cx).await; + let parent = open_parent(&h, vec![agent_says(EXCERPT)], &mut cx); + open_sub_thread(&h, with_context(&parent, EXCERPT), &mut cx); + let first = active_id(&h, &cx); + let first_session = session_of(&h, &cx); + + open_sub_thread(&h, with_context(&parent, "Quincunx pattern"), &mut cx); + let second = active_id(&h, &cx); + + assert_ne!(second, first, "a second thread"); + assert_ne!(session_of(&h, &cx), first_session, "on a second session"); + assert_eq!( + h.connection.forked_from(), + vec![parent.session.clone(), parent.session.clone()] + ); + assert_eq!( + parent_link(first, &mut cx), + Some((parent.id, ForkKind::WithContext)) + ); + assert_eq!( + parent_link(second, &mut cx), + Some((parent.id, ForkKind::WithContext)) + ); + let mut kids = children(parent.id, &mut cx); + kids.sort_by_key(|id| id.to_key_string()); + let mut expected = vec![first, second]; + expected.sort_by_key(|id| id.to_key_string()); + assert_eq!(kids, expected); + assert_eq!( + composer(&h, first, &cx).as_deref(), + Some(QUOTED), + "the first is untouched" + ); +} + +// R3.1, R3.2, R3.3, R3.5 — the parent's agent is asked to fork the parent; the fork opens focused, +// empty, with the quote unsent, linked `with_context` before anything is sent; the parent's +// messages, composer and session are unchanged. +#[gpui::test] +async fn fork_opens_the_forked_session_linked_at_once(cx: &mut TestAppContext) { + let (h, mut cx) = setup(forking(), cx).await; + let parent = open_parent(&h, vec![agent_says(EXCERPT)], &mut cx); + type_text(&h, "a draft the parent keeps", &mut cx); + let parent_entries = entry_count(&parent.thread, &cx); + + open_sub_thread(&h, with_context(&parent, EXCERPT), &mut cx); + + assert_eq!( + h.connection.forked_from(), + vec![parent.session.clone()], + "R3.1: one fork" + ); + let child = active_id(&h, &cx); + let fork = active_thread(&h, &cx); + assert_ne!(child, parent.id, "R3.1: a new thread"); + assert_ne!(fork, parent.thread); + assert_ne!( + session_of(&h, &cx), + parent.session.0.to_string(), + "on the fork's session" + ); + assert_eq!( + active_agent_id(&h, &cx), + AgentId::new("stub"), + "the parent's agent" + ); + assert!(composer_focused(&h, &mut cx), "R3.1: focused"); + assert_eq!( + entry_count(&fork, &cx), + 0, + "the fork opens empty: nothing replayed" + ); + assert_eq!( + composer(&h, child, &cx).as_deref(), + Some(QUOTED), + "R3.2: the quote" + ); + assert_eq!( + parent_link(child, &mut cx), + Some((parent.id, ForkKind::WithContext)), + "R3.3: linked before any send" + ); + assert_eq!( + entry_count(&parent.thread, &cx), + parent_entries, + "R3.5: messages" + ); + assert_eq!( + composer(&h, parent.id, &cx).as_deref(), + Some("a draft the parent keeps"), + "R3.5: composer" + ); + assert_eq!( + parent.thread.read_with(&cx, |t, _| t.session_id().clone()), + parent.session, + "R3.5: session" + ); +} + +/// R3.6's failure, whatever caused it: an `Err`, no thread, no link, one toast, a warning naming +/// the parent session. Both callers fail without an agent error (a stub with no fork, no work +/// dirs), so the warning carries `code=none`. +fn assert_fork_failed( + h: &Harness, + parent: &Parent, + result: anyhow::Result<Entity<ConversationView>>, + (retained_before, toasts_before): (usize, usize), + cx: &mut VisualTestContext, +) { + assert!(result.is_err(), "the open resolves to an error"); + assert_eq!(active_id(h, cx), parent.id, "no thread opened"); + assert_eq!(retained(h, cx), retained_before); + assert!(children(parent.id, cx).is_empty(), "no link"); + assert_eq!(toasts(h, cx), toasts_before + 1, "the toast"); + assert!( + has_fork_failure_line(&parent.session.0), + "a warning names the parent session with code=none: {:?}", + all_lines_lowercased() + ); +} + +// R3.6 — the fork fails (here: an agent without fork). +#[gpui::test] +async fn fork_failure_opens_nothing_and_warns(cx: &mut TestAppContext) { + let (h, mut cx) = setup(StubAgentConnection::new(), cx).await; + let parent = open_parent(&h, vec![agent_says(EXCERPT)], &mut cx); + let before = (retained(&h, &cx), toasts(&h, &cx)); + capture_logs(); + + let result = try_open_sub_thread(&h, with_context(&parent, EXCERPT), true, &mut cx); + + assert_fork_failed(&h, &parent, result, before, &mut cx); +} + +// R3.8 — no work dirs: the same failure, and no fork is even asked of an agent that could fork. +#[gpui::test] +async fn fork_without_work_dirs_fails_before_asking(cx: &mut TestAppContext) { + let (h, mut cx) = setup(forking(), cx).await; + let parent = open_parent(&h, vec![agent_says(EXCERPT)], &mut cx); + let before = (retained(&h, &cx), toasts(&h, &cx)); + capture_logs(); + let mut homeless = with_context(&parent, EXCERPT); + homeless.work_dirs = None; + + let result = try_open_sub_thread(&h, homeless, true, &mut cx); + + assert!( + h.connection.forked_from().is_empty(), + "R3.8: no fork request" + ); + assert_fork_failed(&h, &parent, result, before, &mut cx); +} + +// R3.3 — the fork is not a draft: while it still has no entries its metadata already carries its +// session id (which is what lets the store admit the link), and the view holds the thread the fork +// registered — marked forked from the parent — not one rebuilt by a load. +#[gpui::test] +async fn fork_metadata_carries_the_session_while_empty(cx: &mut TestAppContext) { + let (h, mut cx) = setup(forking(), cx).await; + let parent = open_parent(&h, vec![agent_says(EXCERPT)], &mut cx); + + open_sub_thread(&h, with_context(&parent, EXCERPT), &mut cx); + + let child = active_id(&h, &cx); + let fork = active_thread(&h, &cx); + assert_eq!(entry_count(&fork, &cx), 0, "still empty"); + assert_eq!( + fork.read_with(&cx, |t, _| t.forked_from().cloned()), + Some(parent.session), + "the registered, marked thread" + ); + assert!(!fork.read_with(&cx, |t, _| t.is_draft_thread())); + let saved = cx.update(|_, cx| { + ThreadMetadataStore::global(cx) + .read(cx) + .entry(child) + .and_then(|entry| entry.session_id.clone()) + }); + assert_eq!( + saved, + Some(fork.read_with(&cx, |t, _| t.session_id().clone())), + "saved with its session id while it has no entries" + ); +} + +// `focus: false` — the sub-thread opens, quoted and linked, without taking the focus. +#[gpui::test] +async fn fork_unfocused_open_does_not_take_the_focus(cx: &mut TestAppContext) { + let (h, mut cx) = setup(forking(), cx).await; + let parent = open_parent(&h, vec![agent_says(EXCERPT)], &mut cx); + + let view = try_open_sub_thread(&h, with_context(&parent, EXCERPT), false, &mut cx) + .expect("the fork opens"); + + let thread_view = view + .read_with(&cx, |view, _| view.active_thread().cloned()) + .expect("the fork's thread view"); + let editor = thread_view.read_with(&cx, |view, _| view.message_editor.clone()); + assert!( + !cx.update(|window, cx| editor.focus_handle(cx).contains_focused(window, cx)), + "its composer did not take the focus" + ); + let child = thread_view.read_with(&cx, |view, cx| view.thread.read(cx).session_id().clone()); + assert_ne!(child, parent.session, "a fork was opened all the same"); + assert_eq!(h.connection.forked_from(), vec![parent.session]); +} + +// The forked entity is held only until the view has it: once the sub-thread is closed, nothing +// keeps its thread alive. Control first: a clean sub-thread's thread is released the same way. +#[gpui::test] +async fn fork_forked_thread_is_released_with_its_view(cx: &mut TestAppContext) { + let (h, mut cx) = setup(forking(), cx).await; + let parent = open_parent(&h, vec![agent_says(EXCERPT)], &mut cx); + + open_sub_thread( + &h, + request(&parent, ForkKind::Clean, EXCERPT, None), + &mut cx, + ); + let control = active_thread(&h, &cx).downgrade(); + remove(&h, active_id(&h, &cx), &mut cx); + assert!( + control.upgrade().is_none(), + "precondition: a closed thread is released" + ); + + open_sub_thread(&h, with_context(&parent, EXCERPT), &mut cx); + let fork = active_thread(&h, &cx).downgrade(); + remove(&h, active_id(&h, &cx), &mut cx); + assert!( + fork.upgrade().is_none(), + "no strong reference outlives the view" + ); +} + +// R7.1 — one `zeo_sub_thread event=created` line with both sessions, `kind=with_context` and the +// excerpt's length in characters (12 chars, 16 bytes); never the excerpt's text or a title. +#[gpui::test] +async fn fork_logs_ids_kind_and_length_never_text(cx: &mut TestAppContext) { + let (h, mut cx) = setup(forking(), cx).await; + let parent = open_parent(&h, vec![agent_says("naïve café ☕")], &mut cx); + capture_logs(); + let excerpt = "naïve café ☕"; + + open_sub_thread(&h, with_context(&parent, excerpt), &mut cx); + + let child_session = session_of(&h, &cx); + assert_one_created_line(&parent.session.0, &child_session, "with_context", 12); + for line in all_lines_lowercased() { + assert!( + !line.contains("naïve café"), + "the excerpt reached a log: {line}" + ); + assert!( + !line.contains(&PARENT_TITLE.to_lowercase()), + "a title reached a log: {line}" + ); + } +} diff --git a/crates/agent_ui/src/sub_thread_tests/fork_abandoned.rs b/crates/agent_ui/src/sub_thread_tests/fork_abandoned.rs new file mode 100644 index 0000000..4be9b2e --- /dev/null +++ b/crates/agent_ui/src/sub_thread_tests/fork_abandoned.rs @@ -0,0 +1,155 @@ +// Story 039, task 4.3 review fix F1 — a fork the agent made but the panel could not show (it was +// released between the fork and the open) is closed in the agent, and the open still fails the +// R3.6 way. Filter: `cargo test -p agent_ui sub_thread::fork_abandoned`. +// +// The panel is released between `open_sub_thread` and the first poll of its task: the fork asks +// only the window, which stays, so the stub forks and the open is what fails. + +use acp_thread::StubAgentConnection; +use agent_client_protocol::v2::schema as acp; +use futures::FutureExt as _; +use gpui::{TestAppContext, VisualTestContext}; + +use super::test_support::*; +use crate::thread_organization::ForkKind; + +const EXCERPT: &str = "**Quincunx**"; + +fn forking_and_closing(supports_close: bool) -> StubAgentConnection { + StubAgentConnection::new() + .with_supports_load_session(true) + .with_fork_support() + .with_supports_close_session(supports_close) + // Only the abandoned fork may be closed: the parent's release must not count. + .with_close_sessions_on_release(false) +} + +/// Asks for a fork, releases the panel before the task runs, and returns the connection, the +/// parent's session and what the task resolved to. +async fn fork_then_release_the_panel( + connection: StubAgentConnection, + cx: &mut TestAppContext, +) -> ( + StubAgentConnection, + acp::SessionId, + anyhow::Result<gpui::Entity<crate::ConversationView>>, + VisualTestContext, +) { + let (h, mut cx) = setup(connection, cx).await; + let parent = open_parent(&h, vec![agent_says(EXCERPT)], &mut cx); + h.connection.take_close_session_calls(); + capture_logs(); + let Harness { + panel, + workspace, + connection, + } = h; + + let task = panel.update_in(&mut cx, |panel, window, cx| { + panel.open_sub_thread( + request(&parent, ForkKind::WithContext, EXCERPT, Some(2)), + true, + window, + cx, + ) + }); + let released = panel.downgrade(); + workspace.update_in(&mut cx, |workspace, window, cx| { + workspace.remove_panel(&panel, window, cx) + }); + drop(panel); + settle(&mut cx); + + assert!( + released.upgrade().is_none(), + "fixture: the panel is gone before the fork opens" + ); + assert_eq!( + connection.forked_from(), + vec![parent.session.clone()], + "fixture: the agent forked" + ); + let result = task + .now_or_never() + .expect("open_sub_thread resolves once the window has settled"); + (connection, parent.session, result, cx) +} + +fn abandoned_lines(session: &acp::SessionId) -> Vec<String> { + let session = format!("session={}", session.0.to_lowercase()); + all_lines_lowercased() + .into_iter() + .filter(|line| { + let words: Vec<&str> = line.split_whitespace().collect(); + words.contains(&"event=fork_abandoned") && words.contains(&session.as_str()) + }) + .collect() +} + +#[gpui::test] +async fn fork_abandoned_by_a_released_panel_is_closed_in_the_agent(cx: &mut TestAppContext) { + let (connection, parent_session, result, _cx) = + fork_then_release_the_panel(forking_and_closing(true), cx).await; + + assert!(result.is_err(), "nothing was shown, so the open fails"); + let closed = connection.take_close_session_calls(); + assert_eq!(closed.len(), 1, "the fork is closed once: {closed:?}"); + assert_ne!(closed[0], parent_session, "the fork, never the parent"); + assert!( + has_fork_failure_line(&parent_session.0), + "R3.6's warning is still written" + ); + assert!( + abandoned_lines(&closed[0]) + .iter() + .all(|line| line.split_whitespace().any(|word| word == "close=ok")), + "a successful close is not reported as a failure" + ); +} + +#[gpui::test] +async fn fork_abandoned_close_failure_is_warned_with_the_fork_session(cx: &mut TestAppContext) { + let connection = forking_and_closing(true); + let refusal = connection.defer_next_close_response(); + refusal + .send(Err(anyhow::anyhow!("the agent refused the close"))) + .expect("fixture: the close response is waited on"); + let (connection, parent_session, result, _cx) = + fork_then_release_the_panel(connection, cx).await; + + assert!(result.is_err()); + let closed = connection.take_close_session_calls(); + assert_eq!(closed.len(), 1, "{closed:?}"); + let lines = abandoned_lines(&closed[0]); + assert!( + lines + .iter() + .any(|line| line.split_whitespace().any(|word| word == "close=failed")), + "the refused close names the fork: {:?}", + all_lines_lowercased() + ); + assert!(has_fork_failure_line(&parent_session.0)); +} + +#[gpui::test] +async fn fork_abandoned_without_close_support_is_warned(cx: &mut TestAppContext) { + let (connection, parent_session, result, _cx) = + fork_then_release_the_panel(forking_and_closing(false), cx).await; + + assert!(result.is_err()); + assert!( + connection.take_close_session_calls().is_empty(), + "no close is asked of an agent that cannot close" + ); + let unsupported: Vec<String> = all_lines_lowercased() + .into_iter() + .filter(|line| { + let words: Vec<&str> = line.split_whitespace().collect(); + words.contains(&"event=fork_abandoned") + && words.contains(&"close=unsupported") + && !words.contains(&format!("session={}", parent_session.0.to_lowercase()).as_str()) + }) + .collect(); + assert_eq!(unsupported.len(), 1, "{:?}", all_lines_lowercased()); + assert!(has_fork_failure_line(&parent_session.0)); +} diff --git a/crates/agent_ui/src/sub_thread_tests/header.rs b/crates/agent_ui/src/sub_thread_tests/header.rs new file mode 100644 index 0000000..7da88f6 --- /dev/null +++ b/crates/agent_ui/src/sub_thread_tests/header.rs @@ -0,0 +1,272 @@ +// Story 039, task 5.2 — "Sub-thread of: <parent>" and the inherited band above a sub-thread's +// messages (R4.1–R4.8; Q12). Wiring and the three SEAMS (`header_selector`, `band_selector`, +// `PARENT_BUTTON`): `support.rs`. Filter: `cargo test -p agent_ui sub_thread::header`. +// +// Contract: `render_sub_thread_header(thread_id, inherited: Option<usize>, workspace, cx) -> +// Option<AnyElement>`, placed above the entries by one hook in `ThreadView`'s render, and +// re-rendered because `ConversationView` observes `ThreadOrganizationStore` and +// `ThreadMetadataStore` and notifies its thread view (design.md v2: `ThreadView` observes neither). +// `painted` refreshes the whole window, so the observation itself is asserted by counting the +// thread view's notifications. +// +// Hostile halves first: +// - no link, no header — and a clean draft whose link is still PENDING has none either; +// - N is the count at the moment of the fork: the parent's later turns do not change it. +// The menu-driven case relies on task 5.1's entries. + +use std::cell::Cell; +use std::rc::Rc; + +use gpui::TestAppContext; + +use super::test_support::*; +use crate::thread_organization::{ForkKind, ThreadOrganizationStore}; +use acp_thread::StubAgentConnection; + +const REPLY: &str = "**Quincunx**"; + +fn forking() -> StubAgentConnection { + StubAgentConnection::new() + .with_supports_load_session(true) + .with_fork_support() +} + +fn header_of(title: &str) -> String { + header_selector(&format!("Sub-thread of: {title}")) +} + +fn counted_band(n: usize, title: &str) -> String { + band_selector(&format!("Inherited {n} messages from {title}")) +} + +fn whole_band(title: &str) -> String { + band_selector(&format!("Inherited the conversation of {title}")) +} + +fn deleted_header() -> String { + header_selector("Sub-thread of a deleted thread") +} + +fn no_header_at_all(cx: &mut gpui::VisualTestContext) -> bool { + !painted(&header_of(PARENT_TITLE), cx) + && !painted(&deleted_header(), cx) + && !painted(PARENT_BUTTON, cx) +} + +// Hostile half — R4.7: a thread with no link has no header and no band; a clean sub-thread whose +// link is only pending (no message yet) has none either. +#[gpui::test] +async fn header_hostile_no_link_no_header(cx: &mut TestAppContext) { + let (h, mut cx) = setup(forking(), cx).await; + let parent = open_parent(&h, vec![agent_says(REPLY)], &mut cx); + assert!(no_header_at_all(&mut cx), "the parent itself"); + + open_sub_thread(&h, request(&parent, ForkKind::Clean, REPLY, None), &mut cx); + assert!(no_header_at_all(&mut cx), "a pending link is not a link"); + assert!(!painted(&whole_band(PARENT_TITLE), &mut cx)); +} + +// Hostile half — R4.4: N is counted when the fork is made; two more messages in the parent +// afterwards leave the band at 2. +#[gpui::test] +async fn header_hostile_the_count_is_taken_at_the_moment_of_the_fork(cx: &mut TestAppContext) { + let (h, mut cx) = setup(forking(), cx).await; + let parent = open_parent(&h, vec![agent_says(REPLY)], &mut cx); + select_word("message-content-1-0-0", &mut cx); + right_click("message-content-1-0-0", &mut cx); + click_menu("Sub-thread with context", &mut cx); + let child = active_id(&h, &cx); + assert!( + painted(&counted_band(2, PARENT_TITLE), &mut cx), + "one user + one agent message" + ); + + activate(&h, parent.id, &mut cx); + h.connection + .set_next_prompt_updates(vec![agent_says("More.")]); + send_text(&h, "Anything else?", &mut cx); + activate(&h, child, &mut cx); + + assert!(painted(&counted_band(2, PARENT_TITLE), &mut cx), "still 2"); + assert!( + !painted(&counted_band(4, PARENT_TITLE), &mut cx), + "not the live count" + ); +} + +// R4.1, R4.4 — a forked sub-thread: the header names the parent, the band the known count. +#[gpui::test] +async fn header_with_context_shows_the_parent_and_the_count(cx: &mut TestAppContext) { + let (h, mut cx) = setup(forking(), cx).await; + let parent = open_parent(&h, vec![agent_says(REPLY)], &mut cx); + + open_sub_thread( + &h, + request(&parent, ForkKind::WithContext, REPLY, Some(5)), + &mut cx, + ); + + assert!(painted(&header_of(PARENT_TITLE), &mut cx), "R4.1"); + assert!( + painted(PARENT_BUTTON, &mut cx), + "R4.2: the title is a button" + ); + assert!(painted(&counted_band(5, PARENT_TITLE), &mut cx), "R4.4"); + assert!(!painted(&whole_band(PARENT_TITLE), &mut cx)); +} + +// R4.5 — the count unknown: the band names the conversation instead of a number. +#[gpui::test] +async fn header_with_context_and_unknown_count(cx: &mut TestAppContext) { + let (h, mut cx) = setup(forking(), cx).await; + let parent = open_parent(&h, vec![agent_says(REPLY)], &mut cx); + + open_sub_thread( + &h, + request(&parent, ForkKind::WithContext, REPLY, None), + &mut cx, + ); + + assert!(painted(&whole_band(PARENT_TITLE), &mut cx), "R4.5"); + assert!( + !painted(&counted_band(0, PARENT_TITLE), &mut cx), + "no invented zero" + ); +} + +// R4.6 — a clean sub-thread, once linked, has the header and no band. +#[gpui::test] +async fn header_clean_has_no_band(cx: &mut TestAppContext) { + let (h, mut cx) = setup(forking(), cx).await; + let parent = open_parent(&h, vec![agent_says(REPLY)], &mut cx); + open_sub_thread( + &h, + request(&parent, ForkKind::Clean, REPLY, Some(2)), + &mut cx, + ); + send_text(&h, "Why?", &mut cx); + + assert!(painted(&header_of(PARENT_TITLE), &mut cx), "R4.1"); + assert!(!painted(&counted_band(2, PARENT_TITLE), &mut cx), "R4.6"); + assert!(!painted(&whole_band(PARENT_TITLE), &mut cx), "R4.6"); +} + +// R4.2 — clicking the parent title opens the parent. +#[gpui::test] +async fn header_parent_title_opens_the_parent(cx: &mut TestAppContext) { + let (h, mut cx) = setup(forking(), cx).await; + let parent = open_parent(&h, vec![agent_says(REPLY)], &mut cx); + open_sub_thread( + &h, + request(&parent, ForkKind::WithContext, REPLY, Some(2)), + &mut cx, + ); + + click(PARENT_BUTTON, &mut cx); + + assert_eq!(active_id(&h, &cx), parent.id); +} + +// R4.3 — the parent deleted: the header says so and is not clickable. +#[gpui::test] +async fn header_deleted_parent_is_named_and_not_clickable(cx: &mut TestAppContext) { + let (h, mut cx) = setup(forking(), cx).await; + let parent = open_parent(&h, vec![agent_says(REPLY)], &mut cx); + open_sub_thread( + &h, + request(&parent, ForkKind::WithContext, REPLY, Some(2)), + &mut cx, + ); + let child = active_id(&h, &cx); + + remove(&h, parent.id, &mut cx); + assert_eq!(active_id(&h, &cx), child, "fixture"); + + assert!(painted(&deleted_header(), &mut cx), "R4.3"); + assert!(!painted(PARENT_BUTTON, &mut cx), "not clickable"); + assert!(!painted(&header_of(PARENT_TITLE), &mut cx)); +} + +// R4.8 — the header is re-rendered through `ConversationView`'s observation of both stores: a +// metadata change (the parent renamed) and an organization change (the parent pinned) each +// notify the sub-thread's thread view, with no event of the thread's own. +#[gpui::test] +async fn header_rerenders_on_either_store_change(cx: &mut TestAppContext) { + let (h, mut cx) = setup(forking(), cx).await; + let parent = open_parent(&h, vec![agent_says(REPLY)], &mut cx); + let view = open_sub_thread( + &h, + request(&parent, ForkKind::WithContext, REPLY, Some(2)), + &mut cx, + ); + let thread_view = view + .read_with(&cx, |view, _| view.active_thread().cloned()) + .expect("the sub-thread's thread view"); + let notified = Rc::new(Cell::new(0usize)); + let counter = notified.clone(); + let _watch = + cx.update(|_, cx| cx.observe(&thread_view, move |_, _| counter.set(counter.get() + 1))); + + rename(parent.id, "Ship it", &mut cx); + let after_rename = notified.get(); + assert!( + after_rename > 0, + "a metadata change notifies the thread view" + ); + + cx.update(|_, cx| { + ThreadOrganizationStore::global(cx) + .update(cx, |store, cx| store.pin(parent.id, cx)) + .expect("the parent can be pinned") + }); + settle(&mut cx); + assert!( + notified.get() > after_rename, + "an organization change notifies it too" + ); +} + +// R4.8 — the parent renamed: the header and the band show the new title. +#[gpui::test] +async fn header_follows_a_renamed_parent(cx: &mut TestAppContext) { + let (h, mut cx) = setup(forking(), cx).await; + let parent = open_parent(&h, vec![agent_says(REPLY)], &mut cx); + open_sub_thread( + &h, + request(&parent, ForkKind::WithContext, REPLY, Some(2)), + &mut cx, + ); + + rename(parent.id, "Ship it", &mut cx); + + assert!(painted(&header_of("Ship it"), &mut cx), "R4.8 header"); + assert!(painted(&counted_band(2, "Ship it"), &mut cx), "R4.8 band"); + assert!(!painted(&header_of(PARENT_TITLE), &mut cx)); +} + +// R4.1 — the header sits above the sub-thread's messages. +#[gpui::test] +async fn header_sits_above_the_messages(cx: &mut TestAppContext) { + let (h, mut cx) = setup(forking(), cx).await; + let parent = open_parent(&h, vec![agent_says(REPLY)], &mut cx); + open_sub_thread( + &h, + request(&parent, ForkKind::WithContext, REPLY, Some(2)), + &mut cx, + ); + h.connection + .set_next_prompt_updates(vec![agent_says("Answer.")]); + send_text(&h, "Question?", &mut cx); + + draw(&mut cx); + let header = cx + .debug_bounds(header_of(PARENT_TITLE).leak()) + .expect("the header is painted"); + let reply = cx + .debug_bounds("message-content-1-0-0") + .expect("the sub-thread's reply is painted"); + assert!( + header.origin.y < reply.origin.y, + "{header:?} above {reply:?}" + ); +} diff --git a/crates/agent_ui/src/sub_thread_tests/menu.rs b/crates/agent_ui/src/sub_thread_tests/menu.rs new file mode 100644 index 0000000..2cde5b3 --- /dev/null +++ b/crates/agent_ui/src/sub_thread_tests/menu.rs @@ -0,0 +1,254 @@ +// Story 039, task 5.1 — the two entries in an agent message's context menu (R1.1–R1.8; Q12). +// Wiring: `support.rs`. Filter: `cargo test -p agent_ui sub_thread::menu`. +// +// Driven like upstream's own menu tests (`test_message_context_menu_copies_selected_content`): a +// double-click selects a word of a painted content leaf (`message-content-<entry>-<chunk>-<block>`), +// a right-click opens the menu, and every entry is found by the `MENU_ITEM-<label>` selector +// upstream's `ContextMenu` paints. The parent's reply is one chunk, `**Quincunx**`, so the +// selection's Markdown is `**Quincunx**` and its text `Quincunx`. +// +// Hostile halves first — where the entries must NOT appear: no selection; a selection inside a +// THINKING block, whose content reaches `render_message_context_menu` through the same +// `render_message_content` call site as an agent chunk and is told apart only by the +// `offers_slash_commands` flag that site already carries (`0045`: set for an agent's message, +// never for a thought) — design.md v2 passes it on as the menu's `bool`; the thinking HEADER, +// whose call site passes `false`; the fork entry without the capability; and the fork entry when +// the agent forks but can neither load nor resume (R1.8). The tests drive behaviour only, so they +// hold whatever the flag is called. +// +// Not covered here: the disabled entry's tooltip TEXT, fixed as "Wait for the turn to end" (R1.6; +// a GPUI test cannot read a tooltip's label); the entry is asserted present and inert while the +// turn runs. User messages and tool +// calls never reach this menu (design.md), so they have no UI case. + +use gpui::{BorrowAppContext as _, TestAppContext, VisualTestContext, point, px}; +use settings::SettingsStore; + +use super::test_support::*; +use crate::thread_organization::ForkKind; +use acp_thread::{AgentConnection as _, StubAgentConnection}; +use agent_client_protocol::v1::schema as acp_v1; + +const REPLY: &str = "**Quincunx**"; +const LEAF: &str = "message-content-1-0-0"; +const CLEAN: &str = "Clean sub-thread"; +const FORK: &str = "Sub-thread with context"; + +fn forking() -> StubAgentConnection { + StubAgentConnection::new() + .with_supports_load_session(true) + .with_fork_support() +} + +fn open_menu_on_selection(leaf: &str, cx: &mut VisualTestContext) { + select_word(leaf, cx); + right_click(leaf, cx); + assert!( + menu_has("Copy", cx), + "precondition: the selection reached the menu" + ); +} + +// Hostile half — R1.4: a right-click with no selection offers neither entry. +#[gpui::test] +async fn menu_hostile_no_selection_offers_neither_entry(cx: &mut TestAppContext) { + let (h, mut cx) = setup(forking(), cx).await; + let _parent = open_parent(&h, vec![agent_says(REPLY)], &mut cx); + + right_click(LEAF, &mut cx); + + assert!( + menu_has("Copy This Agent Response", &mut cx), + "precondition: the menu opened" + ); + assert!(!menu_has(CLEAN, &mut cx)); + assert!(!menu_has(FORK, &mut cx)); +} + +// Hostile half — R1.5: text selected in a thinking block, and its header, offer neither entry, +// although the same menu builder serves them. +#[gpui::test] +async fn menu_hostile_a_thinking_block_offers_neither_entry(cx: &mut TestAppContext) { + let (h, mut cx) = setup(forking(), cx).await; + cx.update(|_, cx| { + cx.update_global::<SettingsStore, _>(|store, cx| { + store + .set_user_settings( + r#"{ "agent": { "thinking_display": "always_expanded" } }"#, + cx, + ) + .expect("thinking display setting"); + }) + }); + let _parent = open_parent( + &h, + vec![agent_thinks("**Pondering**"), agent_says(REPLY)], + &mut cx, + ); + let thought = "message-content-1-0-0"; + + open_menu_on_selection(thought, &mut cx); + assert!(!menu_has(CLEAN, &mut cx), "a thinking block's selection"); + assert!(!menu_has(FORK, &mut cx), "a thinking block's selection"); + dismiss_menu(&mut cx); + + // The header row sits right above the thought's content. + draw(&mut cx); + let content = cx.debug_bounds(thought).expect("the thought is painted"); + right_click_at(content.origin + point(px(12.), px(-10.)), &mut cx); + assert!( + menu_has("Copy This Agent Response", &mut cx), + "precondition: the header's menu opened (tune the offset if not)" + ); + assert!(!menu_has(CLEAN, &mut cx), "a thinking header"); + assert!(!menu_has(FORK, &mut cx), "a thinking header"); + + // Control: the agent's own text in the same entry does offer them. + dismiss_menu(&mut cx); + open_menu_on_selection("message-content-1-1-0", &mut cx); + assert!( + menu_has(CLEAN, &mut cx), + "control: the agent text of the same reply" + ); +} + +// Hostile half — R1.3: an agent that does not advertise fork gets the clean entry only. +#[gpui::test] +async fn menu_hostile_without_fork_only_the_clean_entry(cx: &mut TestAppContext) { + let (h, mut cx) = setup(StubAgentConnection::new(), cx).await; + let _parent = open_parent(&h, vec![agent_says(REPLY)], &mut cx); + + open_menu_on_selection(LEAF, &mut cx); + + assert!(menu_has(CLEAN, &mut cx), "R1.1"); + assert!(!menu_has(FORK, &mut cx), "R1.3"); +} + +// Hostile half — R1.8: fork advertised, but the agent can neither load nor resume a session, so +// the panel could not open the fork: the clean entry only. (The stub never resumes; load is off.) +#[gpui::test] +async fn menu_hostile_fork_without_load_or_resume_only_the_clean_entry(cx: &mut TestAppContext) { + let connection = StubAgentConnection::new().with_fork_support(); + assert!( + !connection.supports_load_session() && !connection.supports_resume_session(), + "fixture: neither load nor resume" + ); + let (h, mut cx) = setup(connection, cx).await; + let _parent = open_parent(&h, vec![agent_says(REPLY)], &mut cx); + + open_menu_on_selection(LEAF, &mut cx); + + assert!(menu_has(CLEAN, &mut cx), "R1.1"); + assert!(!menu_has(FORK, &mut cx), "R1.8"); +} + +// R1.1, R1.2, R1.7 — with a selection and the capability: both entries, and the five existing +// ones as before. +#[gpui::test] +async fn menu_with_a_selection_offers_both_and_keeps_the_existing_entries(cx: &mut TestAppContext) { + let (h, mut cx) = setup(forking(), cx).await; + let _parent = open_parent(&h, vec![agent_says(REPLY)], &mut cx); + + open_menu_on_selection(LEAF, &mut cx); + + assert!(menu_has(CLEAN, &mut cx), "R1.1"); + assert!(menu_has(FORK, &mut cx), "R1.2"); + for existing in [ + "Copy", + "Copy as Markdown", + "Copy This Agent Response", + "Open Thread as Markdown", + ] { + assert!(menu_has(existing, &mut cx), "R1.7: {existing}"); + } + assert!( + menu_has("Scroll to Top", &mut cx) || menu_has("Scroll to Bottom", &mut cx), + "R1.7: the scroll entry (its label follows the scroll position)" + ); +} + +// R1.6 — while the parent's turn runs the fork entry is shown but does nothing; once the turn +// ends it forks. The clean entry stays. +#[gpui::test] +async fn menu_fork_entry_is_inert_while_the_turn_runs(cx: &mut TestAppContext) { + let (h, mut cx) = setup(forking(), cx).await; + let parent = open_parent(&h, vec![agent_says(REPLY)], &mut cx); + send_text(&h, "And then?", &mut cx); // no reply queued: the turn stays open + assert_eq!( + parent.thread.read_with(&cx, |t, _| t.status()), + acp_thread::ThreadStatus::Generating, + "fixture: a running turn" + ); + + open_menu_on_selection(LEAF, &mut cx); + assert!(menu_has(CLEAN, &mut cx), "the clean entry is offered"); + assert!(menu_has(FORK, &mut cx), "R1.6: shown"); + click_menu(FORK, &mut cx); + assert!( + h.connection.forked_from().is_empty(), + "R1.6: disabled, nothing forked" + ); + assert_eq!(active_id(&h, &cx), parent.id, "nothing opened"); + + // End the turn BEFORE pressing escape: with the menu already closed, escape would reach the + // thread view and cancel the running turn. + h.connection + .end_turn(parent.session.clone(), acp_v1::StopReason::EndTurn); + settle(&mut cx); + dismiss_menu(&mut cx); + open_menu_on_selection(LEAF, &mut cx); + click_menu(FORK, &mut cx); + assert_eq!( + h.connection.forked_from(), + vec![parent.session.clone()], + "enabled again" + ); +} + +// R1.1 → R2.2 — the clean entry opens the clean sub-thread with the selection's MARKDOWN quoted. +#[gpui::test] +async fn menu_clean_entry_opens_the_quoted_selection(cx: &mut TestAppContext) { + let (h, mut cx) = setup(forking(), cx).await; + let parent = open_parent(&h, vec![agent_says(REPLY)], &mut cx); + + open_menu_on_selection(LEAF, &mut cx); + click_menu(CLEAN, &mut cx); + + let child = active_id(&h, &cx); + assert_ne!(child, parent.id); + assert_eq!( + composer(&h, child, &cx).as_deref(), + Some("> **Quincunx**\n\n") + ); + assert!( + h.connection.forked_from().is_empty(), + "the clean entry does not fork" + ); + assert_eq!( + parent_link(child, &mut cx), + None, + "linked on its first send only" + ); +} + +// R1.2 → R3.1–R3.3 — the fork entry forks the parent and opens the fork, quoted and linked. +#[gpui::test] +async fn menu_fork_entry_opens_the_quoted_fork(cx: &mut TestAppContext) { + let (h, mut cx) = setup(forking(), cx).await; + let parent = open_parent(&h, vec![agent_says(REPLY)], &mut cx); + + open_menu_on_selection(LEAF, &mut cx); + click_menu(FORK, &mut cx); + + let child = active_id(&h, &cx); + assert_ne!(child, parent.id); + assert_eq!(h.connection.forked_from(), vec![parent.session.clone()]); + assert_eq!( + composer(&h, child, &cx).as_deref(), + Some("> **Quincunx**\n\n") + ); + assert_eq!( + parent_link(child, &mut cx), + Some((parent.id, ForkKind::WithContext)) + ); +} diff --git a/crates/agent_ui/src/sub_thread_tests/menu_subagent.rs b/crates/agent_ui/src/sub_thread_tests/menu_subagent.rs new file mode 100644 index 0000000..00d1d40 --- /dev/null +++ b/crates/agent_ui/src/sub_thread_tests/menu_subagent.rs @@ -0,0 +1,187 @@ +// Story 039, task 11.1 — no sub-thread entries in a subagent's thread view (R1.9; refine v5). +// Wiring: `support.rs`, mounted as `mod menu_subagent` beside `mod menu` in `sub_thread.rs`. +// Filter: `cargo test -p agent_ui sub_thread::menu` (selects this file and `menu.rs`). +// +// Why: in a subagent's view the clicked thread's session is the SUBAGENT's while the panel's thread +// id is the root conversation's, so a fork there would inherit one conversation and link another +// (the 6.1 contract check, `.draft/contract-check.md`). +// +// The subagent is reached the way an external agent's subagent reaches Zeo: the root thread +// announces it (`subagent_spawned`, after an Agent tool call carrying `subagent_session_info`), the +// conversation loads its session through the connection, and the conversation navigates to it. +// The subagent's view is told apart by what it shows, never by a field the fixture sets, so the +// test holds whatever the gate reads. +// +// Hostile half first: the subagent's view must offer neither entry although its block is an agent +// message with a selection, exactly like the root's. Its converse: the root view of the SAME +// conversation, before and after the subagent was shown, still offers both — a gate keyed on "this +// conversation has a subagent", or one left set after visiting it, would wrongly remove them there. + +use acp_thread::{AcpThread, StubAgentConnection}; +use agent_client_protocol::{v1::schema as acp_v1, v2::schema as acp}; +use gpui::{Entity, TestAppContext, VisualTestContext}; +use serde_json::json; + +use super::test_support::*; +use crate::ConversationView; + +const REPLY: &str = "**Quincunx**"; +/// The root's reply: entry 0 is the user's message, entry 1 the agent's. +const ROOT_LEAF: &str = "message-content-1-0-0"; +/// The subagent's reply is its thread's first entry. +const SUBAGENT_LEAF: &str = "message-content-0-0-0"; +const SUBAGENT_SESSION: &str = "subagent-session"; +const CLEAN: &str = "Clean sub-thread"; +const FORK: &str = "Sub-thread with context"; + +fn forking() -> StubAgentConnection { + StubAgentConnection::new() + .with_supports_load_session(true) + .with_fork_support() +} + +fn conversation(h: &Harness, cx: &VisualTestContext) -> Entity<ConversationView> { + h.panel + .read_with(cx, |panel, _| panel.active_conversation_view().cloned()) + .expect("a conversation is active") +} + +/// The session of the thread view the conversation shows — the panel's own accessors always +/// answer with the root thread, whichever view is on screen. +fn shown_session(h: &Harness, cx: &VisualTestContext) -> acp::SessionId { + conversation(h, cx).read_with(cx, |view, cx| { + view.active_thread() + .expect("a thread view is shown") + .read(cx) + .thread + .read(cx) + .session_id() + .clone() + }) +} + +/// The root announces a subagent the way an external agent does; its session is loaded into the +/// same conversation and has answered with `REPLY`. +fn spawn_subagent( + h: &Harness, + root: &Entity<AcpThread>, + cx: &mut VisualTestContext, +) -> acp::SessionId { + let session = acp::SessionId::new(SUBAGENT_SESSION); + let mut meta = serde_json::Map::new(); + meta.insert( + acp_thread::SUBAGENT_SESSION_INFO_META_KEY.to_string(), + json!({ "session_id": SUBAGENT_SESSION, "message_start_index": 0 }), + ); + let call = acp_v1::ToolCall::new("toolu_agent", "Agent explore").meta(meta); + root.update(cx, |thread, cx| { + thread + .handle_session_update(acp_v1::SessionUpdate::ToolCall(call), cx) + .expect("the Agent tool call applies"); + }); + let root_session = root.read_with(cx, |thread, _| thread.session_id().clone()); + conversation(h, cx).update(cx, |view, cx| { + view.load_subagent_session_for_test(session.clone(), root_session, cx) + }); + settle(cx); + let loaded = conversation(h, cx).read_with(cx, |view, _| view.thread_view(&session).is_some()); + assert!( + loaded, + "fixture: the subagent's session is loaded into the conversation" + ); + cx.update(|_, cx| { + h.connection + .send_update(session.clone(), agent_says(REPLY), cx) + }); + settle(cx); + session +} + +fn show(h: &Harness, session: &acp::SessionId, cx: &mut VisualTestContext) { + conversation(h, cx).update_in(cx, |view, window, cx| { + view.navigate_to_thread(session.clone(), window, cx) + }); + settle(cx); + assert_eq!( + &shown_session(h, cx), + session, + "fixture: that thread is on screen" + ); +} + +fn open_menu_on_selection(leaf: &str, cx: &mut VisualTestContext) { + select_word(leaf, cx); + right_click(leaf, cx); + assert!( + menu_has("Copy", cx), + "precondition: the selection reached the menu" + ); +} + +fn assert_existing_entries(cx: &mut VisualTestContext, which: &str) { + for existing in [ + "Copy", + "Copy as Markdown", + "Copy This Agent Response", + "Open Thread as Markdown", + ] { + assert!(menu_has(existing, cx), "R1.7 in {which}: {existing}"); + } + assert!( + menu_has("Scroll to Top", cx) || menu_has("Scroll to Bottom", cx), + "R1.7 in {which}: the scroll entry (its label follows the scroll position)" + ); +} + +// Hostile half — R1.9: a selection in a subagent's agent message offers neither entry, and the +// five existing entries stay. +#[gpui::test] +async fn menu_subagent_view_offers_neither_entry(cx: &mut TestAppContext) { + let (h, mut cx) = setup(forking(), cx).await; + let root = open_parent(&h, vec![agent_says(REPLY)], &mut cx); + let subagent = spawn_subagent(&h, &root.thread, &mut cx); + assert_ne!(subagent, root.session, "fixture: two sessions"); + + show(&h, &subagent, &mut cx); + open_menu_on_selection(SUBAGENT_LEAF, &mut cx); + + assert!(!menu_has(CLEAN, &mut cx), "R1.9: a subagent's view"); + assert!(!menu_has(FORK, &mut cx), "R1.9: a subagent's view"); + assert_existing_entries(&mut cx, "a subagent's view"); + assert!( + h.connection.forked_from().is_empty(), + "nothing was forked from the subagent" + ); +} + +// Converse — the root view of the same conversation keeps both entries, before the subagent is +// shown and after the view comes back from it (R1.1, R1.2, R1.9). +#[gpui::test] +async fn menu_subagent_root_view_still_offers_both_entries(cx: &mut TestAppContext) { + let (h, mut cx) = setup(forking(), cx).await; + let root = open_parent(&h, vec![agent_says(REPLY)], &mut cx); + let subagent = spawn_subagent(&h, &root.thread, &mut cx); + assert_eq!( + shown_session(&h, &cx), + root.session, + "fixture: the root is shown" + ); + + open_menu_on_selection(ROOT_LEAF, &mut cx); + assert!(menu_has(CLEAN, &mut cx), "root view, subagent loaded: R1.1"); + assert!(menu_has(FORK, &mut cx), "root view, subagent loaded: R1.2"); + assert_existing_entries(&mut cx, "the root view"); + dismiss_menu(&mut cx); + + show(&h, &subagent, &mut cx); + show(&h, &root.session, &mut cx); + open_menu_on_selection(ROOT_LEAF, &mut cx); + assert!( + menu_has(CLEAN, &mut cx), + "root view, back from the subagent: R1.1" + ); + assert!( + menu_has(FORK, &mut cx), + "root view, back from the subagent: R1.2" + ); +} diff --git a/crates/agent_ui/src/sub_thread_tests/pure.rs b/crates/agent_ui/src/sub_thread_tests/pure.rs new file mode 100644 index 0000000..48fe8b8 --- /dev/null +++ b/crates/agent_ui/src/sub_thread_tests/pure.rs @@ -0,0 +1,221 @@ +// Story 039, task 4.1 — the two pure helpers of `crates/agent_ui/src/sub_thread.rs` (R2.2, R3.2, +// R4.4; Q5). Wiring: see `support.rs` (this module needs none of it). Filter: +// `cargo test -p agent_ui sub_thread::pure`. +// +// Contract (design.md *Sub-thread orchestration*): +// pub fn quote_excerpt(excerpt: &str) -> String — the selection's trailing whitespace trimmed +// first; then every line prefixed `> ` (an empty line becomes `>`); then `\n\n`. +// pub fn inherited_message_count(entries: &[AgentThreadEntry]) -> usize — user and agent +// messages; tool calls and thinking-only entries are not messages. +// +// Hostile halves first: +// - quoting must not collapse what the selection keeps apart: an interior blank line stays a +// line of its own (`>`), and only the selection's END is trimmed — an interior line's two +// trailing spaces (a Markdown hard break) and a code line's leading indentation survive; +// - counting must neither collapse nor split: a thought-only entry and a tool call are not +// messages, and an agent entry holding a thought AND text is ONE message, not two. + +use std::path::Path; +use std::rc::Rc; + +use acp_thread::{AcpThread, AgentConnection as _, AgentThreadEntry, StubAgentConnection}; +use agent_client_protocol::v1::schema as acp_v1; +use fs::FakeFs; +use gpui::{Entity, TestAppContext}; +use project::Project; +use util::path_list::PathList; + +use super::{inherited_message_count, quote_excerpt}; + +// ── quote_excerpt ────────────────────────────────────────────────────────── + +#[test] +fn pure_quote_hostile_an_interior_blank_line_stays_a_bare_marker() { + assert_eq!(quote_excerpt("first\n\nsecond"), "> first\n>\n> second\n\n"); + assert!( + !quote_excerpt("first\n\nsecond").contains("> \n"), + "an empty line is `>`, with no trailing space" + ); +} + +#[test] +fn pure_quote_hostile_only_the_selection_end_is_trimmed() { + // "line " ends in a Markdown hard break: it is inside the selection, so it stays. + assert_eq!( + quote_excerpt("line \nnext \n\n \t"), + "> line \n> next\n\n", + "trailing whitespace of the selection is trimmed, an interior line's is kept" + ); + assert_eq!( + quote_excerpt(" let x = 1;\n x"), + "> let x = 1;\n> x\n\n", + "leading indentation is kept" + ); +} + +#[test] +fn pure_quote_one_line() { + assert_eq!(quote_excerpt("Ship it"), "> Ship it\n\n"); +} + +#[test] +fn pure_quote_several_lines() { + assert_eq!( + quote_excerpt("Tag the release.\nThen publish."), + "> Tag the release.\n> Then publish.\n\n" + ); +} + +#[test] +fn pure_quote_keeps_markdown_verbatim() { + assert_eq!( + quote_excerpt("**Bump** `zeo` to [0.6](https://zeoeditor.dev)\n- then tag"), + "> **Bump** `zeo` to [0.6](https://zeoeditor.dev)\n> - then tag\n\n" + ); +} + +#[test] +fn pure_quote_always_ends_with_exactly_one_empty_line() { + for excerpt in ["a", "a\nb", "a\n\n\n", "a\r\n"] { + let quoted = quote_excerpt(excerpt); + assert!(quoted.ends_with("\n\n"), "{excerpt:?} -> {quoted:?}"); + assert!(!quoted.ends_with("\n\n\n"), "{excerpt:?} -> {quoted:?}"); + } +} + +// ── inherited_message_count ──────────────────────────────────────────────── + +fn user(text: &str) -> acp_v1::SessionUpdate { + acp_v1::SessionUpdate::UserMessageChunk(acp_v1::ContentChunk::new(text.into())) +} + +fn says(text: &str) -> acp_v1::SessionUpdate { + acp_v1::SessionUpdate::AgentMessageChunk(acp_v1::ContentChunk::new(text.into())) +} + +fn thinks(text: &str) -> acp_v1::SessionUpdate { + acp_v1::SessionUpdate::AgentThoughtChunk(acp_v1::ContentChunk::new(text.into())) +} + +fn tool(id: &str) -> acp_v1::SessionUpdate { + acp_v1::SessionUpdate::ToolCall(acp_v1::ToolCall::new( + acp_v1::ToolCallId::new(id.to_string()), + "Read the changelog", + )) +} + +/// A thread whose transcript is exactly `updates`, as an agent would stream (or replay) them. +async fn thread_of( + updates: Vec<acp_v1::SessionUpdate>, + cx: &mut TestAppContext, +) -> Entity<AcpThread> { + crate::test_support::init_test(cx); + let project = Project::test(FakeFs::new(cx.executor()), [], cx).await; + let connection = Rc::new(StubAgentConnection::new()); + let thread = cx + .update(|cx| { + connection + .clone() + .new_session(project, PathList::new(&[Path::new("/project")]), cx) + }) + .await + .expect("a stub session"); + let session_id = thread.read_with(cx, |thread, _| thread.session_id().clone()); + for update in updates { + cx.update(|cx| connection.send_update(session_id.clone(), update, cx)); + } + cx.run_until_parked(); + thread +} + +/// The entries' shapes: "user", "agent" (with text), "thought" (thought only), "tool". +fn shapes(thread: &Entity<AcpThread>, cx: &TestAppContext) -> Vec<&'static str> { + thread.read_with(cx, |thread, _| { + thread + .entries() + .iter() + .map(|entry| match entry { + AgentThreadEntry::UserMessage(_) => "user", + AgentThreadEntry::AssistantMessage(message) => { + if message.chunks.iter().any(|chunk| { + matches!(chunk, acp_thread::AssistantMessageChunk::Message { .. }) + }) { + "agent" + } else { + "thought" + } + } + AgentThreadEntry::ToolCall(_) => "tool", + _ => "other", + }) + .collect() + }) +} + +fn count(thread: &Entity<AcpThread>, cx: &TestAppContext) -> usize { + thread.read_with(cx, |thread, _| inherited_message_count(thread.entries())) +} + +#[gpui::test] +async fn pure_count_hostile_thoughts_and_tool_calls_are_not_messages(cx: &mut TestAppContext) { + let thread = thread_of( + vec![ + user("Plan it"), + thinks("Weighing it"), + tool("t1"), + says("Done"), + ], + cx, + ) + .await; + assert_eq!( + shapes(&thread, cx), + ["user", "thought", "tool", "agent"], + "fixture" + ); + assert_eq!(count(&thread, cx), 2); +} + +#[gpui::test] +async fn pure_count_hostile_one_agent_entry_is_one_message_whatever_its_chunks( + cx: &mut TestAppContext, +) { + let thread = thread_of( + vec![ + user("Plan it"), + thinks("Weighing it"), + says("Step one."), + says(" Step two."), + ], + cx, + ) + .await; + assert_eq!( + shapes(&thread, cx), + ["user", "agent"], + "fixture: one agent entry" + ); + assert_eq!(count(&thread, cx), 2, "per entry, not per chunk"); +} + +#[gpui::test] +async fn pure_count_users_and_agents_across_turns(cx: &mut TestAppContext) { + let thread = thread_of( + vec![user("One"), says("Two"), user("Three"), says("Four")], + cx, + ) + .await; + assert_eq!( + shapes(&thread, cx), + ["user", "agent", "user", "agent"], + "fixture" + ); + assert_eq!(count(&thread, cx), 4); +} + +#[gpui::test] +async fn pure_count_an_empty_thread_is_zero(cx: &mut TestAppContext) { + let thread = thread_of(Vec::new(), cx).await; + assert_eq!(count(&thread, cx), 0); + assert_eq!(inherited_message_count(&[]), 0); +} diff --git a/crates/agent_ui/src/sub_thread_tests/reopen.rs b/crates/agent_ui/src/sub_thread_tests/reopen.rs new file mode 100644 index 0000000..05f51bd --- /dev/null +++ b/crates/agent_ui/src/sub_thread_tests/reopen.rs @@ -0,0 +1,276 @@ +// Story 039, task 5.4 — a never-used fork that cannot be reopened (R4.9; Q4). Wiring: `support.rs`, +// plus, in `crates/agent_ui/src/sub_thread.rs`: +// #[cfg(test)] +// #[path = "sub_thread_tests/reopen.rs"] +// mod reopen; // 5.4 +// Filter: `cargo test -p agent_ui sub_thread::reopen`. +// +// Contract (design.md v2 *Header and band*, never-used fork): when reopening a thread fails, +// `ConversationView`'s load-result match checks — before the error becomes a `LoadError` — +// whether it downcasts to `acp::Error` with code `ResourceNotFound` (-32002) AND the organization +// store links this thread `with_context`. Only then it shows the `SubThreadUnavailable` state: the +// header plus "This sub-thread was never used and can't be reopened". Any other case keeps +// upstream's `handle_load_error` (`ConversationView::load_error()` is `Some`). Row and link stay. +// +// Why it happens (story.md, Constraints): the adapter resumes a fork's SDK session to load it, and +// a fork never prompted has no transcript, so a reload after a restart answers resource-not-found. +// Here a fake connection answers every `session/load` with the chosen code. +// +// SEAM — the one element design.md names no selector for: the state's text, found as +// `SUB_THREAD_UNAVAILABLE-<the whole text>` (adjust `unavailable_selector` if it is named +// otherwise). The header keeps support.rs's `SUB_THREAD_HEADER-<text>`. +// +// Hostile halves first — the state needs BOTH conditions: the same -32002 on an unlinked or a +// clean-linked thread, and another code on a with-context thread, all keep the generic error. + +use std::path::Path; +use std::rc::Rc; + +use acp_thread::{AcpThread, AgentConnection, StubAgentConnection}; +use agent_client_protocol::v2::schema as acp; +use chrono::Utc; +use gpui::{App, Entity, SharedString, Task, TestAppContext, VisualTestContext}; +use project::{AgentId, Project, WorktreePaths}; +use util::path_list::PathList; + +use super::test_support::*; +use crate::test_support::StubAgentServer; +use crate::thread_metadata_store::{ThreadId, ThreadMetadata, ThreadMetadataStore}; +use crate::thread_organization::{ForkKind, ThreadOrganizationStore}; +use crate::{Agent, AgentThreadSource}; + +const NOT_FOUND: i32 = -32002; +const INTERNAL: i32 = -32603; +const UNAVAILABLE_TEXT: &str = "This sub-thread was never used and can't be reopened"; + +fn unavailable_selector() -> String { + format!("SUB_THREAD_UNAVAILABLE-{UNAVAILABLE_TEXT}") +} + +/// The Plus adapter reloading a session it cannot find: every `session/load` answers an +/// `acp::Error` with `code`, wrapped in `anyhow` as `map_acp_error` wraps it. +#[derive(Clone)] +struct RefusingReloadConnection { + code: i32, +} + +impl AgentConnection for RefusingReloadConnection { + fn agent_id(&self) -> AgentId { + AgentId::new("stub") + } + + fn telemetry_id(&self) -> SharedString { + "stub".into() + } + + fn new_session( + self: Rc<Self>, + _project: Entity<Project>, + _work_dirs: PathList, + _cx: &mut App, + ) -> Task<anyhow::Result<Entity<AcpThread>>> { + Task::ready(Err(anyhow::anyhow!("these tests only reopen threads"))) + } + + fn supports_load_session(&self) -> bool { + true + } + + fn load_session( + self: Rc<Self>, + _session_id: acp::SessionId, + _project: Entity<Project>, + _work_dirs: PathList, + _title: Option<SharedString>, + _cx: &mut App, + ) -> Task<anyhow::Result<Entity<AcpThread>>> { + let error = agent_client_protocol::Error::new(self.code, "Resource not found"); + Task::ready(Err(anyhow::Error::new(error))) + } + + fn auth_methods(&self) -> &[acp::AuthMethod] { + &[] + } + + fn authenticate(&self, _method: acp::AuthMethodId, _cx: &mut App) -> Task<anyhow::Result<()>> { + Task::ready(Ok(())) + } + + fn cancel(&self, _session_id: &acp::SessionId, _cx: &mut App) -> anyhow::Result<()> { + Ok(()) + } + + fn into_any(self: Rc<Self>) -> Rc<dyn std::any::Any> { + self + } +} + +/// A panel whose `Agent::Stub` answers every reload with `code`. +async fn refusing_panel(code: i32, cx: &mut TestAppContext) -> (Harness, VisualTestContext) { + let (h, mut cx) = setup(StubAgentConnection::new(), cx).await; + let store = h + .panel + .read_with(&cx, |panel, _| panel.connection_store().clone()); + store.update(&mut cx, |store, cx| { + store.restart_connection( + Agent::Stub, + Rc::new( + StubAgentServer::new(RefusingReloadConnection { code }).with_connection_agent_id(), + ), + cx, + ); + }); + cx.run_until_parked(); + (h, cx) +} + +/// A saved thread of `PROJECT` on the stub agent, with a session (a sent message or a fork). +fn saved_thread(session: &str, title: &str, cx: &mut VisualTestContext) -> ThreadId { + let now = Utc::now(); + let metadata = ThreadMetadata { + thread_id: ThreadId::new(), + session_id: Some(acp::SessionId::new(session)), + agent_id: AgentId::new("stub"), + title: Some(title.to_string().into()), + title_override: None, + updated_at: now, + created_at: Some(now), + interacted_at: None, + worktree_paths: WorktreePaths::from_folder_paths(&PathList::new(&[Path::new(PROJECT)])), + remote_connection: None, + archived: false, + }; + let id = metadata.thread_id; + cx.update(|_, cx| { + ThreadMetadataStore::global(cx).update(cx, |store, cx| store.save(metadata, cx)) + }); + cx.run_until_parked(); + id +} + +fn link(child: ThreadId, parent: ThreadId, kind: ForkKind, cx: &mut VisualTestContext) { + cx.update(|_, cx| { + ThreadOrganizationStore::global(cx).update(cx, |store, cx| { + store + .set_parent(child, parent, kind, cx) + .expect("two saved threads can be linked") + }) + }); + cx.run_until_parked(); +} + +/// Reopens `id` the way the sidebar and the history do. +fn reopen(h: &Harness, id: ThreadId, cx: &mut VisualTestContext) { + h.panel.update_in(cx, |panel, window, cx| { + panel.load_agent_thread( + Agent::Stub, + id, + Some(PathList::new(&[Path::new(PROJECT)])), + None, + true, + AgentThreadSource::Sidebar, + window, + cx, + ) + }); + settle(cx); +} + +fn generic_load_error(h: &Harness, cx: &VisualTestContext) -> bool { + h.panel.read_with(cx, |panel, cx| { + panel + .active_conversation_view() + .is_some_and(|view| view.read(cx).load_error().is_some()) + }) +} + +struct Fixture { + h: Harness, + cx: VisualTestContext, + parent: ThreadId, + child: ThreadId, +} + +/// A parent and a child saved on the stub agent, the child linked with `kind` (or not), then +/// reopened on an agent answering `code`. +async fn reopened(code: i32, kind: Option<ForkKind>, cx: &mut TestAppContext) -> Fixture { + let (h, mut cx) = refusing_panel(code, cx).await; + let parent = saved_thread("parent-session", PARENT_TITLE, &mut cx); + let child = saved_thread("never-used-fork", "Forkling", &mut cx); + if let Some(kind) = kind { + link(child, parent, kind, &mut cx); + } + reopen(&h, child, &mut cx); + assert_eq!( + active_id(&h, &cx), + child, + "fixture: the child is the one reopened" + ); + Fixture { + h, + cx, + parent, + child, + } +} + +// Hostile half — the same -32002 on a thread with no link is an ordinary load failure. +#[gpui::test] +async fn reopen_hostile_unlinked_thread_keeps_the_generic_error(cx: &mut TestAppContext) { + let Fixture { h, mut cx, .. } = reopened(NOT_FOUND, None, cx).await; + assert!(generic_load_error(&h, &cx), "upstream's load error"); + assert!(!painted(&unavailable_selector(), &mut cx)); +} + +// Hostile half — a CLEAN sub-thread inherited nothing to lose: generic error, no special state. +#[gpui::test] +async fn reopen_hostile_clean_sub_thread_keeps_the_generic_error(cx: &mut TestAppContext) { + let Fixture { h, mut cx, .. } = reopened(NOT_FOUND, Some(ForkKind::Clean), cx).await; + assert!(generic_load_error(&h, &cx), "upstream's load error"); + assert!(!painted(&unavailable_selector(), &mut cx)); +} + +// Hostile half — a with-context sub-thread failing with ANOTHER code is an ordinary failure. +#[gpui::test] +async fn reopen_hostile_other_error_code_keeps_the_generic_error(cx: &mut TestAppContext) { + let Fixture { h, mut cx, .. } = reopened(INTERNAL, Some(ForkKind::WithContext), cx).await; + assert!(generic_load_error(&h, &cx), "upstream's load error"); + assert!(!painted(&unavailable_selector(), &mut cx)); +} + +// R4.9 — a with-context sub-thread whose reload is not found: its own state with the header, and +// its row and link kept. +#[gpui::test] +async fn reopen_never_used_fork_shows_its_own_state(cx: &mut TestAppContext) { + let Fixture { + h, + mut cx, + parent, + child, + } = reopened(NOT_FOUND, Some(ForkKind::WithContext), cx).await; + + assert!( + painted(&unavailable_selector(), &mut cx), + "R4.9: the state's text" + ); + assert!( + painted( + &header_selector(&format!("Sub-thread of: {PARENT_TITLE}")), + &mut cx + ), + "R4.9: the header stays" + ); + assert_eq!( + parent_link(child, &mut cx), + Some((parent, ForkKind::WithContext)), + "R4.9: the link stays" + ); + assert!( + cx.update(|_, cx| ThreadMetadataStore::global(cx) + .read(cx) + .entry(child) + .is_some()), + "R4.9: the row stays" + ); + assert_eq!(active_id(&h, &cx), child, "nothing else was opened instead"); +} diff --git a/crates/agent_ui/src/sub_thread_tests/support.rs b/crates/agent_ui/src/sub_thread_tests/support.rs new file mode 100644 index 0000000..5a539cc --- /dev/null +++ b/crates/agent_ui/src/sub_thread_tests/support.rs @@ -0,0 +1,500 @@ +// Story 039 — shared fixtures for the sub-thread tests of tasks 4.2–5.4 (task 4.1's `pure` needs +// none of them). One file per sub-task, so a sub-task's Green never depends on a later sub-task's +// API (a test module naming a missing item breaks the whole crate's test build), and no file +// passes 500 lines. +// +// Wired INSIDE `crates/agent_ui/src/sub_thread.rs`, so the test paths match the Validation +// filters `sub_thread::{pure,clean,fork,menu,header,reopen}`: +// #[cfg(test)] +// #[path = "sub_thread_tests/pure.rs"] +// mod pure; // 4.1 +// #[cfg(test)] +// #[path = "sub_thread_tests/support.rs"] +// mod test_support; // with 4.2 +// #[cfg(test)] +// #[path = "sub_thread_tests/clean.rs"] +// mod clean; // 4.2 +// #[cfg(test)] +// #[path = "sub_thread_tests/fork.rs"] +// mod fork; // 4.3 +// #[cfg(test)] +// #[path = "sub_thread_tests/menu.rs"] +// mod menu; // 5.1 +// #[cfg(test)] +// #[path = "sub_thread_tests/header.rs"] +// mod header; // 5.2 +// #[cfg(test)] +// #[path = "sub_thread_tests/reopen.rs"] +// mod reopen; // 5.4 +// (tasks.md names one file, `sub_thread_tests.rs`; it is split for the two reasons above.) +// +// Contract (design.md *Sub-thread orchestration*): `SubThreadRequest { parent_thread_id, +// parent_session_id, agent, work_dirs, kind, excerpt, inherited_messages }` and +// `AgentPanel::open_sub_thread(&mut self, request, focus: bool, window, cx) -> +// Task<Result<Entity<ConversationView>>>`. +// +// One stub for every thread: the parent, the clean sub-threads and the forks all run on ONE +// `StubAgentConnection` registered as `Agent::Stub` in the panel's connection store — as every +// thread of one external agent shares one `AcpConnection` in Zeo — so the session a fork +// registers is the one the forked view attaches to. +// +// SEAMS — the only lines naming elements design.md does not name (adjust HERE if the +// implementation names them otherwise; each is a debug selector): +// `header_selector(text)` "SUB_THREAD_HEADER-<the header's whole text>" +// `band_selector(text)` "SUB_THREAD_BAND-<the band's whole text>" +// `PARENT_BUTTON` "SUB_THREAD_PARENT", the clickable parent title +// The menu needs no seam: upstream's `ContextMenu` paints every entry as `MENU_ITEM-<label>`. + +use std::path::Path; +use std::rc::Rc; +use std::time::Duration; + +use acp_thread::{AcpThread, AgentThreadEntry, StubAgentConnection}; +use agent_client_protocol::{v1::schema as acp_v1, v2::schema as acp}; +use fs::FakeFs; +use futures::FutureExt as _; +use gpui::{ + AppContext as _, Entity, Focusable as _, Modifiers, MouseButton, SharedString, TestAppContext, + VisualTestContext, point, px, +}; +use project::{AgentId, Project}; +use serde_json::json; +use util::path_list::PathList; +use workspace::{MultiWorkspace, Workspace}; + +use super::SubThreadRequest; +use crate::test_support::StubAgentServer; +use crate::thread_metadata_store::{ThreadId, ThreadMetadataStore}; +use crate::thread_organization::{ForkKind, ThreadOrganizationStore}; +use crate::{Agent, AgentPanel, ConversationView, NewExternalAgentThread}; + +pub const PROJECT: &str = "/project"; +pub const PARENT_TITLE: &str = "Plan the release"; + +pub fn header_selector(text: &str) -> String { + format!("SUB_THREAD_HEADER-{text}") +} + +pub fn band_selector(text: &str) -> String { + format!("SUB_THREAD_BAND-{text}") +} + +pub const PARENT_BUTTON: &str = "SUB_THREAD_PARENT"; + +pub struct Harness { + pub panel: Entity<AgentPanel>, + pub workspace: Entity<Workspace>, + pub connection: StubAgentConnection, +} + +pub struct Parent { + pub id: ThreadId, + pub session: acp::SessionId, + pub thread: Entity<AcpThread>, +} + +/// A visible agent panel on `PROJECT`, the organization store installed as Zeo installs it, and +/// `connection` serving `Agent::Stub`. +pub async fn setup( + connection: StubAgentConnection, + cx: &mut TestAppContext, +) -> (Harness, VisualTestContext) { + crate::conversation_view::tests::init_test(cx); + cx.update(|cx| { + cx.bind_keys([gpui::KeyBinding::new("escape", menu::Cancel, Some("menu"))]); // no keymap + agent::ThreadStore::init_global(cx); + language_model::LanguageModelRegistry::test(cx); + crate::thread_organization::init(cx); + }); + let fs = FakeFs::new(cx.executor()); + cx.update(|cx| <dyn fs::Fs>::set_global(fs.clone(), cx)); + fs.insert_tree(PROJECT, json!({ "file.txt": "" })).await; + let project = Project::test(fs.clone(), [Path::new(PROJECT)], cx).await; + let window = cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + let workspace = window + .read_with(cx, |multi_workspace, _| multi_workspace.workspace().clone()) + .expect("the window's workspace"); + let mut vcx = VisualTestContext::from_window(window.into(), cx); + vcx.simulate_resize(gpui::size(px(1200.), px(800.))); + let panel = workspace.update_in(&mut vcx, |workspace, window, cx| { + let panel = cx.new(|cx| AgentPanel::new(workspace, window, cx)); + workspace.add_panel(panel.clone(), window, cx); + workspace.focus_panel::<AgentPanel>(window, cx); + panel + }); + vcx.run_until_parked(); + let store = panel.read_with(&vcx, |panel, _| panel.connection_store().clone()); + store.update(&mut vcx, |store, cx| { + store.restart_connection( + Agent::Stub, + Rc::new(StubAgentServer::new(connection.clone()).with_connection_agent_id()), + cx, + ); + }); + vcx.run_until_parked(); + ( + Harness { + panel, + workspace, + connection, + }, + vcx, + ) +} + +pub fn agent_says(text: &str) -> acp_v1::SessionUpdate { + acp_v1::SessionUpdate::AgentMessageChunk(acp_v1::ContentChunk::new(text.into())) +} + +pub fn agent_thinks(text: &str) -> acp_v1::SessionUpdate { + acp_v1::SessionUpdate::AgentThoughtChunk(acp_v1::ContentChunk::new(text.into())) +} + +/// Lets deferred work (a would-be auto-submit, a deferred store write) run out. +pub fn settle(cx: &mut VisualTestContext) { + cx.run_until_parked(); + cx.executor().advance_clock(Duration::from_secs(5)); + cx.run_until_parked(); +} + +pub fn draw(cx: &mut VisualTestContext) { + cx.update(|window, _| window.refresh()); + cx.run_until_parked(); +} + +/// Types `text` into the active thread's composer and sends it. +pub fn send_text(h: &Harness, text: &str, cx: &mut VisualTestContext) { + let view = h + .panel + .read_with(cx, |panel, cx| panel.active_thread_view(cx)) + .expect("an active thread view"); + let editor = view.read_with(cx, |view, _| view.message_editor.clone()); + editor.update_in(cx, |editor, window, cx| editor.set_text(text, window, cx)); + view.update_in(cx, |view, window, cx| view.send(window, cx)); + settle(cx); +} + +/// Types `text` into the active composer without sending it. +pub fn type_text(h: &Harness, text: &str, cx: &mut VisualTestContext) { + let view = h + .panel + .read_with(cx, |panel, cx| panel.active_thread_view(cx)) + .expect("an active thread view"); + let editor = view.read_with(cx, |view, _| view.message_editor.clone()); + editor.update_in(cx, |editor, window, cx| editor.set_text(text, window, cx)); + cx.run_until_parked(); +} + +/// A thread on `Agent::Stub` that has had one turn answered with `reply`, titled `PARENT_TITLE`. +pub fn open_parent( + h: &Harness, + reply: Vec<acp_v1::SessionUpdate>, + cx: &mut VisualTestContext, +) -> Parent { + h.panel.update_in(cx, |panel, window, cx| { + panel.new_external_agent_thread( + &NewExternalAgentThread { + agent: AgentId::new("stub"), + }, + window, + cx, + ) + }); + cx.run_until_parked(); + h.connection.set_next_prompt_updates(reply); + send_text(h, "Plan the release, please.", cx); + let id = active_id(h, cx); + let thread = active_thread(h, cx); + let session = thread.read_with(cx, |thread, _| thread.session_id().clone()); + rename(id, PARENT_TITLE, cx); + assert!( + entry_count(&thread, cx) >= 2, + "fixture: the parent has a turn" + ); + Parent { + id, + session, + thread, + } +} + +pub fn request( + parent: &Parent, + kind: ForkKind, + excerpt: &str, + inherited_messages: Option<usize>, +) -> SubThreadRequest { + SubThreadRequest { + parent_thread_id: parent.id, + parent_session_id: parent.session.clone(), + agent: Agent::Stub, + work_dirs: Some(PathList::new(&[Path::new(PROJECT)])), + kind, + excerpt: SharedString::from(excerpt.to_string()), + inherited_messages, + } +} + +/// Opens a sub-thread and returns what `open_sub_thread`'s task resolved to, once the panel has +/// settled (the stub answers at once, so the task is done by then). +pub fn try_open_sub_thread( + h: &Harness, + request: SubThreadRequest, + focus: bool, + cx: &mut VisualTestContext, +) -> anyhow::Result<Entity<ConversationView>> { + let task = h.panel.update_in(cx, |panel, window, cx| { + panel.open_sub_thread(request, focus, window, cx) + }); + settle(cx); + task.now_or_never() + .expect("open_sub_thread resolves once the panel has settled") +} + +/// A focused open that must succeed; the returned conversation is the active one. +pub fn open_sub_thread( + h: &Harness, + request: SubThreadRequest, + cx: &mut VisualTestContext, +) -> Entity<ConversationView> { + let view = try_open_sub_thread(h, request, true, cx).expect("the sub-thread opens"); + assert_eq!( + h.panel + .read_with(cx, |panel, _| panel.active_conversation_view().cloned()), + Some(view.clone()), + "the returned conversation is the one shown" + ); + view +} + +// ── Reads ────────────────────────────────────────────────────────────────── + +pub fn active_id(h: &Harness, cx: &VisualTestContext) -> ThreadId { + h.panel + .read_with(cx, |panel, cx| panel.active_thread_id(cx)) + .expect("an active thread") +} + +pub fn active_thread(h: &Harness, cx: &VisualTestContext) -> Entity<AcpThread> { + h.panel + .read_with(cx, |panel, cx| panel.active_agent_thread(cx)) + .expect("an active agent thread") +} + +pub fn active_agent_id(h: &Harness, cx: &VisualTestContext) -> AgentId { + h.panel.read_with(cx, |panel, cx| { + panel + .active_conversation_view() + .expect("a conversation is active") + .read(cx) + .agent_key() + .id() + }) +} + +pub fn composer(h: &Harness, id: ThreadId, cx: &VisualTestContext) -> Option<String> { + h.panel.read_with(cx, |panel, cx| panel.editor_text(id, cx)) +} + +pub fn composer_focused(h: &Harness, cx: &mut VisualTestContext) -> bool { + let view = h + .panel + .read_with(cx, |panel, cx| panel.active_thread_view(cx)) + .expect("an active thread view"); + let editor = view.read_with(cx, |view, _| view.message_editor.clone()); + cx.update(|window, cx| editor.focus_handle(cx).contains_focused(window, cx)) +} + +pub fn entry_count(thread: &Entity<AcpThread>, cx: &VisualTestContext) -> usize { + thread.read_with(cx, |thread, _| thread.entries().len()) +} + +pub fn user_messages(thread: &Entity<AcpThread>, cx: &VisualTestContext) -> usize { + thread.read_with(cx, |thread, _| { + thread + .entries() + .iter() + .filter(|entry| matches!(entry, AgentThreadEntry::UserMessage(_))) + .count() + }) +} + +pub fn parent_link(child: ThreadId, cx: &mut VisualTestContext) -> Option<(ThreadId, ForkKind)> { + cx.update(|_, cx| ThreadOrganizationStore::global(cx).read(cx).parent(child)) +} + +pub fn children(parent: ThreadId, cx: &mut VisualTestContext) -> Vec<ThreadId> { + cx.update(|_, cx| { + ThreadOrganizationStore::global(cx) + .read(cx) + .children(parent) + }) +} + +pub fn toasts(h: &Harness, cx: &VisualTestContext) -> usize { + h.workspace + .read_with(cx, |workspace, _| workspace.notification_ids().len()) +} + +pub fn retained(h: &Harness, cx: &VisualTestContext) -> usize { + h.panel + .read_with(cx, |panel, _| panel.retained_threads().len()) +} + +pub fn rename(id: ThreadId, title: &str, cx: &mut VisualTestContext) { + let title = SharedString::from(title.to_string()); + cx.update(|_, cx| { + ThreadMetadataStore::global(cx) + .update(cx, |store, cx| store.set_title_override(id, title, cx)) + }); + cx.run_until_parked(); +} + +pub fn activate(h: &Harness, id: ThreadId, cx: &mut VisualTestContext) { + h.panel.update_in(cx, |panel, window, cx| { + panel.activate_retained_thread(id, true, window, cx) + }); + cx.run_until_parked(); + assert_eq!(active_id(h, cx), id, "fixture: the thread is active again"); +} + +pub fn remove(h: &Harness, id: ThreadId, cx: &mut VisualTestContext) { + h.panel + .update_in(cx, |panel, window, cx| panel.remove_thread(id, window, cx)); + settle(cx); +} + +// ── The rendered panel ───────────────────────────────────────────────────── + +pub fn painted(selector: &str, cx: &mut VisualTestContext) -> bool { + draw(cx); + cx.debug_bounds(selector.to_string().leak()).is_some() +} + +/// Double-clicks a word of the content leaf `selector` (upstream's selection recipe, +/// `test_message_context_menu_copies_selected_content`). +pub fn select_word(selector: &str, cx: &mut VisualTestContext) { + draw(cx); + let bounds = cx + .debug_bounds(selector.to_string().leak()) + .expect("the content leaf is painted"); + let position = bounds.origin + point(px(12.), bounds.size.height / 2.); + cx.simulate_event(gpui::MouseDownEvent { + position, + button: MouseButton::Left, + modifiers: Modifiers::default(), + click_count: 2, + first_mouse: false, + }); + cx.simulate_mouse_up(position, MouseButton::Left, Modifiers::default()); + cx.run_until_parked(); +} + +pub fn right_click_at(position: gpui::Point<gpui::Pixels>, cx: &mut VisualTestContext) { + cx.simulate_mouse_down(position, MouseButton::Right, Modifiers::default()); + cx.simulate_mouse_up(position, MouseButton::Right, Modifiers::default()); + draw(cx); +} + +pub fn right_click(selector: &str, cx: &mut VisualTestContext) { + draw(cx); + let bounds = cx + .debug_bounds(selector.to_string().leak()) + .expect("the content leaf is painted"); + right_click_at(bounds.origin + point(px(12.), bounds.size.height / 2.), cx); +} + +pub fn menu_has(label: &str, cx: &mut VisualTestContext) -> bool { + cx.debug_bounds(format!("MENU_ITEM-{label}").leak()) + .is_some() +} + +pub fn click_menu(label: &str, cx: &mut VisualTestContext) { + let item = cx + .debug_bounds(format!("MENU_ITEM-{label}").leak()) + .unwrap_or_else(|| panic!("the menu shows {label}")); + cx.simulate_click(item.center(), Modifiers::default()); + settle(cx); +} + +pub fn dismiss_menu(cx: &mut VisualTestContext) { + // No frame loop here, and `right_click_menu` focuses a menu on its second frame. + for _ in 0..2 { + cx.update(|window, cx| window.simulate_next_frame(cx)); + } + cx.simulate_keystrokes("escape"); + draw(cx); +} + +pub fn click(selector: &str, cx: &mut VisualTestContext) { + draw(cx); + let bounds = cx + .debug_bounds(selector.to_string().leak()) + .unwrap_or_else(|| panic!("{selector} is painted")); + cx.simulate_click(bounds.center(), Modifiers::default()); + settle(cx); +} + +// ── Log lines (the capture of patch 0048's test helpers) ─────────────────── + +pub(crate) use crate::thread_organization::test_helpers::{all_lines_lowercased, capture_logs}; + +/// The line's words, split on anything that is not a letter, a digit, '-' or '_'. +pub fn tokens(line: &str) -> Vec<&str> { + line.split(|c: char| !(c.is_alphanumeric() || c == '-' || c == '_')) + .filter(|token| !token.is_empty()) + .collect() +} + +/// Captured lines that carry every one of `words` as a whole token. +pub fn lines_with_tokens(words: &[&str]) -> Vec<String> { + all_lines_lowercased() + .into_iter() + .filter(|line| { + let tokens = tokens(line); + words + .iter() + .all(|word| tokens.contains(&word.to_lowercase().as_str())) + }) + .collect() +} + +/// R7.1's line, `zeo_sub_thread event=created parent_session=<id> session=<id> +/// kind=<with_context|clean> excerpt_chars=<n>`: exactly one, with exactly these values. Read +/// word by word, so `session=` never matches inside `parent_session=`. +pub fn assert_one_created_line(parent_session: &str, session: &str, kind: &str, chars: usize) { + let created: Vec<Vec<String>> = all_lines_lowercased() + .into_iter() + .map(|line| line.split_whitespace().map(str::to_string).collect()) + .filter(|words: &Vec<String>| { + words.iter().any(|w| w == "zeo_sub_thread") + && words.iter().any(|w| w == "event=created") + }) + .collect(); + assert_eq!( + created.len(), + 1, + "one created line per sub-thread: {:?}", + all_lines_lowercased() + ); + for expected in [ + format!("parent_session={}", parent_session.to_lowercase()), + format!("session={}", session.to_lowercase()), + format!("kind={kind}"), + format!("excerpt_chars={chars}"), + ] { + assert!( + created[0].contains(&expected), + "{expected} missing from {:?}", + created[0] + ); + } +} + +/// A captured warning about a failed fork naming `parent_session` and carrying `code=none` +/// (R3.6: not an agent error). +pub fn has_fork_failure_line(parent_session: &str) -> bool { + lines_with_tokens(&[parent_session]) + .iter() + .any(|line| line.split_whitespace().any(|word| word == "code=none")) +} diff --git a/crates/sidebar/src/sidebar.rs b/crates/sidebar/src/sidebar.rs index 361a42d..6e62920 100644 --- a/crates/sidebar/src/sidebar.rs +++ b/crates/sidebar/src/sidebar.rs @@ -89,6 +89,9 @@ mod sidebar_tests; #[cfg(test)] mod sidebar_pinned_tests; +#[cfg(test)] +mod sidebar_sub_thread_tests; +mod sub_thread_marker; mod organization; #[cfg(test)] @@ -6787,6 +6790,7 @@ impl Sidebar { .when_some(thread.pinned_project_label.clone(), |this, label| { this.project_name(label) }) + .map(|item| sub_thread_marker::mark_sub_thread(item, thread.metadata.thread_id, cx)) .tags(thread_tags::tag_chips_for(thread.metadata.thread_id, cx)) .timestamp(timestamp) .highlight_positions(thread.highlight_positions.to_vec()) diff --git a/crates/sidebar/src/sidebar_sub_thread_tests.rs b/crates/sidebar/src/sidebar_sub_thread_tests.rs new file mode 100644 index 0000000..aac9e48 --- /dev/null +++ b/crates/sidebar/src/sidebar_sub_thread_tests.rs @@ -0,0 +1,328 @@ +// Story 039, task 5.3 — the "↳" marker on a sub-thread's row in the threads sidebar (R5.1–R5.5; +// Q5). +// +// Materialize with, in `crates/sidebar/src/sidebar.rs`, beside `mod sidebar_pinned_tests;` (0049, +// applied before this patch) — never beside `mod sidebar_devtools_tests;`, which 0063 adds after +// it: +// #[cfg(test)] +// mod sidebar_sub_thread_tests; +// Filter: `cargo test -p sidebar sub_thread` (paths `sidebar_sub_thread_tests::…`). +// +// Contract (design.md *Sidebar marker*): `crate::sub_thread_marker::sub_thread_badge(thread_id, +// cx) -> Option<(IconName, SharedString)>` — `Some((<an arrow icon>, "Sub-thread of <parent +// title>"))` for a linked thread, the tooltip "Sub-thread of a deleted thread" when the parent's +// metadata is gone, `None` otherwise; `render_thread` hands it to story 037's badge. +// +// Uses only 039's API. The one exception is the PAINTED check (`marker_painted`), the only line +// naming the badge's debug selector: `THREAD_ITEM_BADGE`, which `ThreadItem` paints at the series +// tip (0063). If story 037's generalised badge renames it, adjust it there. +// +// Reuses story 032's `pub(super)` helpers from `sidebar_tests.rs`. Links are written through the +// real store (`ThreadOrganizationStore::set_parent`); a restart is `thread_organization::init` +// again over the same per-test database, then a new window (story 032's recipe). +// +// Hostile halves first: +// - the marker follows the LINK, not a title: an unlinked thread titled like a sub-thread gets +// none, and the tooltip names the PARENT's title, never the row's own; +// - a link moves nothing and adds nothing: the rows, in order, are exactly those of the same +// threads unlinked (no indent, no second row under the parent); +// - the marker is not the devtools badge (a different icon). + +use super::*; +use crate::sidebar_tests::{ + init_test, save_thread_metadata_with_main_paths, setup_sidebar, visible_entries_as_strings, +}; +use agent_ui::thread_metadata_store::{ThreadId, ThreadMetadataStore}; +use agent_ui::thread_organization::{ForkKind, ThreadOrganizationStore}; +use fs::FakeFs; +use gpui::{TestAppContext, VisualTestContext}; +use std::cell::Cell; +use std::rc::Rc; +use ui::IconName; +use util::path_list::PathList; + +use crate::sub_thread_marker::sub_thread_badge; + +const ALPHA: &str = "/alpha"; + +async fn init_alpha(cx: &mut TestAppContext) -> Entity<project::Project> { + init_test(cx); + let fs = FakeFs::new(cx.executor()); + fs.insert_tree(ALPHA, serde_json::json!({ "src": {} })) + .await; + cx.update(|cx| { + <dyn fs::Fs>::set_global(fs.clone(), cx); + agent_ui::thread_organization::init(cx); + }); + cx.run_until_parked(); + project::Project::test(fs, [ALPHA.as_ref()], cx).await +} + +/// A window on `project` with its sidebar open. +fn window( + project: Entity<project::Project>, + cx: &mut TestAppContext, +) -> (Entity<Sidebar>, VisualTestContext) { + let (multi_workspace, vcx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project, window, cx)); + let sidebar = setup_sidebar(&multi_workspace, vcx); + (sidebar, vcx.clone()) +} + +/// A thread with a sent message in `ALPHA`; a later `minute` sorts higher. +fn thread(session: &str, title: &str, minute: u32, cx: &mut TestAppContext) -> ThreadId { + let paths = PathList::new(&[ALPHA]); + save_thread_metadata_with_main_paths( + session, + title, + paths.clone(), + paths, + chrono::TimeZone::with_ymd_and_hms(&chrono::Utc, 2026, 10, 20, 12, minute, 0).unwrap(), + cx, + ); + cx.update(|cx| { + ThreadMetadataStore::global(cx) + .read(cx) + .entries() + .find(|e| { + e.session_id + .as_ref() + .is_some_and(|s| s.0.as_ref() == session) + }) + .map(|e| e.thread_id) + .expect("saved") + }) +} + +fn link(child: ThreadId, parent: ThreadId, kind: ForkKind, cx: &mut TestAppContext) { + cx.update(|cx| { + ThreadOrganizationStore::global(cx).update(cx, |store, cx| { + store + .set_parent(child, parent, kind, cx) + .expect("two saved threads can be linked") + }) + }); + cx.run_until_parked(); +} + +fn badge(id: ThreadId, cx: &mut TestAppContext) -> Option<(IconName, String)> { + cx.update(|cx| sub_thread_badge(id, cx)) + .map(|(icon, tip)| (icon, tip.to_string())) +} + +fn tooltip(id: ThreadId, cx: &mut TestAppContext) -> Option<String> { + badge(id, cx).map(|(_, tip)| tip) +} + +/// The thread rows, trimmed, in order. +fn rows(sidebar: &Entity<Sidebar>, cx: &mut VisualTestContext) -> Vec<String> { + visible_entries_as_strings(sidebar, cx) + .into_iter() + .filter(|row| row.starts_with(" ")) + .map(|row| row.trim().to_string()) + .collect() +} + +/// `(title, tooltip)` for every thread row the sidebar lists, in order. +fn row_markers( + sidebar: &Entity<Sidebar>, + cx: &mut VisualTestContext, +) -> Vec<(String, Option<String>)> { + let ids: Vec<(String, ThreadId)> = sidebar.read_with(cx, |sidebar, _| { + sidebar + .contents + .entries + .iter() + .filter_map(|entry| match entry { + ListEntry::Thread(t) => { + Some((t.metadata.display_title().to_string(), t.metadata.thread_id)) + } + _ => None, + }) + .collect() + }); + ids.into_iter() + .map(|(title, id)| (title, tooltip(id, cx))) + .collect() +} + +/// SEAM — whether a badge is painted anywhere in this window's sidebar. +fn marker_painted(cx: &mut VisualTestContext) -> bool { + cx.update(|window, _| window.refresh()); + cx.run_until_parked(); + cx.debug_bounds("THREAD_ITEM_BADGE").is_some() +} + +// Hostile half — the marker follows the link, not a title, and names the PARENT. +#[gpui::test] +async fn sub_thread_hostile_marker_follows_the_link_and_names_the_parent(cx: &mut TestAppContext) { + let project = init_alpha(cx).await; + let (sidebar, mut vcx) = window(project, cx); + let parent = thread("s-parent", "Plan the release", 1, cx); + let lookalike = thread("s-lookalike", "Sub-thread of Plan the release", 2, cx); + let child = thread("s-child", "Why ship first", 3, cx); + + link(child, parent, ForkKind::Clean, cx); + + assert_eq!(tooltip(lookalike, cx), None, "R5.2: a title is not a link"); + assert_eq!( + tooltip(parent, cx), + None, + "R5.2: the parent is not a sub-thread" + ); + assert_eq!( + tooltip(child, cx).as_deref(), + Some("Sub-thread of Plan the release"), + "R5.1: the parent's title, not the row's own" + ); + assert_eq!( + row_markers(&sidebar, &mut vcx) + .into_iter() + .filter(|(_, tip)| tip.is_some()) + .map(|(title, _)| title) + .collect::<Vec<_>>(), + vec!["Why ship first".to_string()], + "exactly one row carries the marker" + ); +} + +// Hostile half — a link moves nothing and adds nothing: same rows, same order, one row each. +#[gpui::test] +async fn sub_thread_hostile_a_linked_row_keeps_its_usual_place(cx: &mut TestAppContext) { + let project = init_alpha(cx).await; + let (sidebar, mut vcx) = window(project, cx); + let parent = thread("s-parent", "Plan the release", 1, cx); + let _middle = thread("s-middle", "Unrelated work", 2, cx); + let child = thread("s-child", "Why ship first", 3, cx); + let before = rows(&sidebar, &mut vcx); + + link(child, parent, ForkKind::WithContext, cx); + + assert_eq!( + rows(&sidebar, &mut vcx), + before, + "R5.1: a normal row at its usual place" + ); +} + +// The marker is a marker of its own, not the devtools badge. +#[gpui::test] +async fn sub_thread_hostile_marker_is_not_the_devtools_badge(cx: &mut TestAppContext) { + let _project = init_alpha(cx).await; + let parent = thread("s-parent", "Plan the release", 1, cx); + let child = thread("s-child", "Why ship first", 2, cx); + link(child, parent, ForkKind::WithContext, cx); + + let (icon, _) = badge(child, cx).expect("a marker"); + assert_ne!(icon, IconName::ToolWeb, "the devtools badge's icon"); +} + +// R5.1, R5.2 — painted for a linked row, not painted when no row is linked. +#[gpui::test] +async fn sub_thread_marker_is_painted_only_with_a_link(cx: &mut TestAppContext) { + let project = init_alpha(cx).await; + let (_sidebar, mut vcx) = window(project, cx); + let parent = thread("s-parent", "Plan the release", 1, cx); + let child = thread("s-child", "Why ship first", 2, cx); + assert!(!marker_painted(&mut vcx), "R5.2: no link, no marker"); + + link(child, parent, ForkKind::Clean, cx); + + assert!(marker_painted(&mut vcx), "R5.1"); +} + +// R5.5 — the parent deleted: the sub-thread keeps its row and its marker, with the deleted wording. +#[gpui::test] +async fn sub_thread_survives_a_deleted_parent(cx: &mut TestAppContext) { + let project = init_alpha(cx).await; + let (sidebar, mut vcx) = window(project, cx); + let parent = thread("s-parent", "Plan the release", 1, cx); + let child = thread("s-child", "Why ship first", 2, cx); + link(child, parent, ForkKind::WithContext, cx); + + cx.update(|cx| { + ThreadMetadataStore::global(cx).update(cx, |store, cx| store.delete(parent, cx)) + }); + cx.run_until_parked(); + + assert_eq!( + rows(&sidebar, &mut vcx), + vec!["Why ship first".to_string()], + "the row stays" + ); + assert_eq!( + tooltip(child, cx).as_deref(), + Some("Sub-thread of a deleted thread") + ); +} + +// The tooltip follows a renamed parent. +#[gpui::test] +async fn sub_thread_tooltip_follows_a_renamed_parent(cx: &mut TestAppContext) { + let _project = init_alpha(cx).await; + let parent = thread("s-parent", "Plan the release", 1, cx); + let child = thread("s-child", "Why ship first", 2, cx); + link(child, parent, ForkKind::Clean, cx); + + cx.update(|cx| { + ThreadMetadataStore::global(cx).update(cx, |store, cx| { + store.set_title_override(parent, "Ship it".into(), cx) + }) + }); + cx.run_until_parked(); + + assert_eq!(tooltip(child, cx).as_deref(), Some("Sub-thread of Ship it")); +} + +// R5.3 — a link recorded with two windows open reaches the other window's sidebar without a +// restart: it re-renders and paints the marker. +#[gpui::test] +async fn sub_thread_marker_reaches_every_window(cx: &mut TestAppContext) { + let project = init_alpha(cx).await; + let (_sidebar_a, _vcx_a) = window(project, cx); + let fs = cx.update(|cx| <dyn fs::Fs>::global(cx)); + let other = project::Project::test(fs, [ALPHA.as_ref()], cx).await; + let (sidebar_b, mut vcx_b) = window(other, cx); + let parent = thread("s-parent", "Plan the release", 1, cx); + let child = thread("s-child", "Why ship first", 2, cx); + assert!(!marker_painted(&mut vcx_b), "fixture: nothing linked yet"); + let notified = Rc::new(Cell::new(0usize)); + let counter = notified.clone(); + let _watch = cx.update(|cx| cx.observe(&sidebar_b, move |_, _| counter.set(counter.get() + 1))); + + link(child, parent, ForkKind::WithContext, cx); + + assert!(notified.get() > 0, "R5.3: window B's sidebar re-rendered"); + assert!(marker_painted(&mut vcx_b), "R5.3: and paints the marker"); +} + +// R5.4 — after a restart (a new store over the same database, a new window) the marker is back. +#[gpui::test] +async fn sub_thread_marker_survives_a_restart(cx: &mut TestAppContext) { + let project = init_alpha(cx).await; + let (_sidebar, _vcx) = window(project, cx); + let parent = thread("s-parent", "Plan the release", 1, cx); + let child = thread("s-child", "Why ship first", 2, cx); + link(child, parent, ForkKind::Clean, cx); + + cx.run_until_parked(); + cx.update(|cx| agent_ui::thread_organization::init(cx)); + cx.run_until_parked(); + let fs = cx.update(|cx| <dyn fs::Fs>::global(cx)); + let reopened = project::Project::test(fs, [ALPHA.as_ref()], cx).await; + let (sidebar, mut vcx) = window(reopened, cx); + + assert_eq!( + tooltip(child, cx).as_deref(), + Some("Sub-thread of Plan the release"), + "R5.4: the link was read back" + ); + assert!(marker_painted(&mut vcx), "R5.4: and painted"); + assert_eq!( + row_markers(&sidebar, &mut vcx) + .into_iter() + .filter(|(_, tip)| tip.is_some()) + .count(), + 1 + ); +} diff --git a/crates/sidebar/src/sub_thread_marker.rs b/crates/sidebar/src/sub_thread_marker.rs new file mode 100644 index 0000000..867bc34 --- /dev/null +++ b/crates/sidebar/src/sub_thread_marker.rs @@ -0,0 +1,39 @@ +//! The "↳" marker on a sub-thread's row in the threads sidebar (story 039, R5). `sidebar.rs` only +//! hands the result to `ThreadItem::badge` through one hook line in `render_thread`. + +use agent_ui::ThreadId; +use agent_ui::thread_metadata_store::ThreadMetadataStore; +use agent_ui::thread_organization::ThreadOrganizationStore; +use gpui::{App, SharedString}; +use ui::{IconName, ThreadItem}; + +/// The tree has no `CornerDownRight`; `ArrowDownRight` is the closest existing glyph to "↳", and +/// design.md rules out a new SVG. +const SUB_THREAD_ICON: IconName = IconName::ArrowDownRight; + +/// The marker for a thread with a parent link: the icon and a tooltip naming the parent's title, +/// or the deleted wording once the parent's metadata is gone (R5.5). `None` for a thread with no +/// link (R5.2), and without the organization store, where no link can be recorded. +/// +/// The link lives in the global store every window's sidebar observes, which is what makes it +/// show in every window (R5.3) and survive a restart (R5.4) without any state here. +pub(crate) fn sub_thread_badge(thread_id: ThreadId, cx: &App) -> Option<(IconName, SharedString)> { + let store = ThreadOrganizationStore::try_global(cx)?; + let (parent, _kind) = store.read(cx).parent(thread_id)?; + let parent_title = ThreadMetadataStore::try_global(cx) + .and_then(|metadata| Some(metadata.read(cx).entry(parent)?.display_title())); + let tooltip = match parent_title { + Some(title) => format!("Sub-thread of {title}"), + None => "Sub-thread of a deleted thread".to_string(), + }; + Some((SUB_THREAD_ICON, tooltip.into())) +} + +/// `render_thread`'s hook: adds the marker after any badge already on the row. `ThreadItem` keeps +/// its badges in a list, so a devtools sub-thread shows both, devtools first. +pub(crate) fn mark_sub_thread(item: ThreadItem, thread_id: ThreadId, cx: &App) -> ThreadItem { + match sub_thread_badge(thread_id, cx) { + Some((icon, tooltip)) => item.badge(icon, tooltip), + None => item, + } +}