diff --git a/crates/op-host-desktop/src/design_session.rs b/crates/op-host-desktop/src/design_session.rs index 7061d51f0..dd686c145 100644 --- a/crates/op-host-desktop/src/design_session.rs +++ b/crates/op-host-desktop/src/design_session.rs @@ -555,6 +555,10 @@ mod tests; #[path = "design_session_worker_tests.rs"] mod worker_tests; +#[cfg(test)] +#[path = "design_session_terminal_tests.rs"] +mod terminal_tests; + /// Render a provider quota-exhaustion error (HTTP 429 with an /// `AccountQuotaExceeded`-style body) as one human sentence instead of /// raw JSON. Extracts the reset timestamp when the provider names one diff --git a/crates/op-host-desktop/src/design_session_terminal_tests.rs b/crates/op-host-desktop/src/design_session_terminal_tests.rs new file mode 100644 index 000000000..b1a1a2b26 --- /dev/null +++ b/crates/op-host-desktop/src/design_session_terminal_tests.rs @@ -0,0 +1,275 @@ +use super::*; +use op_editor_core::{ChatActivityStatus, ChatCompletion, ChatMessage, Locale}; +use op_editor_host_core::chat::{apply_poll_to_message, ChatPoll}; +use op_editor_host_core::design::{DesignCmdReq, DesignDelta}; +use op_host_native::WidgetHostNative; +use op_orchestrator::{Progress, RunSummary, SubtaskOutcome}; +use std::sync::mpsc; + +fn failed_subtask(id: &str, label: &str) -> op_orchestrator::plan::Subtask { + op_orchestrator::plan::Subtask { + id: id.into(), + label: label.into(), + region: op_orchestrator::plan::Region { + width: 375.0, + height: 180.0, + }, + id_prefix: id.into(), + parent_frame_id: Some("root".into()), + elements: None, + screen: Some("Now".into()), + generated_root_id: None, + existing_section_labels: None, + retry_feedback: None, + } +} + +fn design_request_json() -> String { + serde_json::to_string(&op_orchestrator::DesignRequest { + prompt: "design a weather app".into(), + model: Some("gemini-3.6-flash".into()), + provider: Some("antigravity".into()), + design_md: None, + concurrency: 1, + append_context: None, + validation_enabled: true, + visual_ref_enabled: false, + }) + .unwrap() +} + +fn failed_run_summary() -> RunSummary { + RunSummary { + root_frame_id: "root".into(), + subtasks: vec![ + SubtaskOutcome { + id: "hero".into(), + node_count: 12, + error: None, + inserted_root_ids: vec!["hero-root".into()], + subtask: None, + }, + SubtaskOutcome { + id: "sun_arc".into(), + node_count: 0, + error: Some("self-check failed".into()), + inserted_root_ids: Vec::new(), + subtask: Some(failed_subtask("sun_arc", "Sunrise & Sunset Arc")), + }, + ], + total_nodes: 12, + unfilled_screens: Vec::new(), + } +} + +#[test] +fn companion_chat_disconnect_cannot_drop_validation_completion_or_retry_payload() { + let (delta_tx, delta_rx) = mpsc::channel::(); + let (_cmd_tx, cmd_rx) = mpsc::channel::(); + let mut current = Some(DesignSession::from_channels(delta_rx, cmd_rx)); + let mut host = WidgetHostNative::new(); + host.editor_state_mut().editor_ui.locale = Locale::EnUs; + host.editor_state_mut() + .chat + .messages + .push(ChatMessage::user("design a weather app")); + let mut primary = ChatMessage::assistant_streaming(); + primary.design_request_json_for_retry = Some(design_request_json()); + host.editor_state_mut().chat.messages.push(primary); + + for event in [ + Progress::Planned { + subtasks: vec![ + ("hero".into(), "Weather Hero".into()), + ("sun_arc".into(), "Sunrise & Sunset Arc".into()), + ], + }, + Progress::SubtaskDone { + id: "hero".into(), + node_count: 12, + }, + Progress::SubtaskFailed { + id: "sun_arc".into(), + error: "self-check failed".into(), + }, + Progress::CleanupDone, + Progress::ValidationStarted, + ] { + delta_tx.send(DesignDelta::Progress(event)).unwrap(); + } + assert!(pump_progress(&mut host, &mut current, None)); + + // This is the real CLI-standard terminal ordering: its companion chat + // sender drops after run_design_worker returns, while the design channel + // already contains ValidationDone + Done. The app pumps chat first. + let primary = host.editor_state_mut().chat.messages.last_mut().unwrap(); + apply_poll_to_message( + primary, + &ChatPoll { + text: String::new(), + thinking: String::new(), + tool_calls: Vec::new(), + error: None, + finished: true, + }, + ); + assert!(!primary.streaming); + + delta_tx + .send(DesignDelta::Progress(Progress::ValidationPreCheckDone { + applied: 1, + by_category: Default::default(), + })) + .unwrap(); + delta_tx + .send(DesignDelta::Progress(Progress::ValidationDone { + total_applied: 1, + })) + .unwrap(); + delta_tx + .send(DesignDelta::Done(Ok(failed_run_summary()))) + .unwrap(); + + assert!(pump_progress(&mut host, &mut current, None)); + assert!(current.is_none()); + let message = host.editor_state().chat.messages.last().unwrap(); + assert_eq!( + message + .activities + .iter() + .find(|activity| activity.id == "__validation") + .unwrap() + .status, + ChatActivityStatus::Done + ); + assert_eq!( + message + .activities + .iter() + .find(|activity| activity.id == "sun_arc") + .unwrap() + .status, + ChatActivityStatus::Error + ); + assert_eq!(message.failed_subtasks.len(), 1); + assert_eq!(message.failed_subtasks[0].subtask_id, "sun_arc"); + assert_eq!( + message.completion, + Some(ChatCompletion { + succeeded: 1, + failed: 1, + nodes: 12, + }) + ); +} + +#[test] +fn first_design_pump_after_chat_disconnect_keeps_all_progress_and_retry_payload() { + let (delta_tx, delta_rx) = mpsc::channel::(); + let (_cmd_tx, cmd_rx) = mpsc::channel::(); + let mut current = Some(DesignSession::from_channels(delta_rx, cmd_rx)); + let mut host = WidgetHostNative::new(); + host.editor_state_mut().editor_ui.locale = Locale::EnUs; + host.editor_state_mut() + .chat + .messages + .push(ChatMessage::user("design a weather app")); + let mut primary = ChatMessage::assistant_streaming(); + primary.design_request_json_for_retry = Some(design_request_json()); + host.editor_state_mut().chat.messages.push(primary); + + for event in [ + Progress::Planning, + Progress::Planned { + subtasks: vec![ + ("hero".into(), "Weather Hero".into()), + ("sun_arc".into(), "Sunrise & Sunset Arc".into()), + ], + }, + Progress::SubtaskDone { + id: "hero".into(), + node_count: 12, + }, + Progress::SubtaskFailed { + id: "sun_arc".into(), + error: "self-check failed".into(), + }, + Progress::CleanupDone, + Progress::ValidationStarted, + Progress::ValidationDone { total_applied: 0 }, + ] { + delta_tx.send(DesignDelta::Progress(event)).unwrap(); + } + delta_tx + .send(DesignDelta::Done(Ok(failed_run_summary()))) + .unwrap(); + + // A fast design can finish before the first design pump. The companion + // chat is still pumped first, so the request marker must route the first + // design progress event even though no activity exists yet. + let primary = host.editor_state_mut().chat.messages.last_mut().unwrap(); + apply_poll_to_message( + primary, + &ChatPoll { + text: String::new(), + thinking: String::new(), + tool_calls: Vec::new(), + error: None, + finished: true, + }, + ); + assert!(!primary.streaming); + + assert!(pump_progress(&mut host, &mut current, None)); + assert!(current.is_none()); + let message = host.editor_state().chat.messages.last().unwrap(); + assert_eq!( + message + .activities + .iter() + .find(|activity| activity.id == "__validation") + .unwrap() + .status, + ChatActivityStatus::Done + ); + assert_eq!( + message + .activities + .iter() + .find(|activity| activity.id == "sun_arc") + .unwrap() + .status, + ChatActivityStatus::Error + ); + assert_eq!(message.failed_subtasks.len(), 1); + assert_eq!(message.failed_subtasks[0].subtask_id, "sun_arc"); +} + +#[test] +fn unused_design_session_disconnect_does_not_finish_plain_chat_bubble() { + let (delta_tx, delta_rx) = mpsc::channel::(); + let (cmd_tx, cmd_rx) = mpsc::channel::(); + drop(delta_tx); + drop(cmd_tx); + let mut current = Some(DesignSession::from_channels(delta_rx, cmd_rx)); + let mut host = WidgetHostNative::new(); + host.editor_state_mut() + .chat + .messages + .push(ChatMessage::user("explain this")); + host.editor_state_mut().chat.messages.push({ + let mut message = ChatMessage::assistant_streaming(); + // CLI-standard stashes this before route classification, including + // for Chat and Modify turns. It is not proof that the parked + // DesignSession owns the bubble. + message.design_request_json_for_retry = Some(design_request_json()); + message + }); + + assert!(!pump_progress(&mut host, &mut current, None)); + assert!(current.is_none()); + assert!( + host.editor_state().chat.messages.last().unwrap().streaming, + "the unused parked DesignSession must not terminate the real ChatSession bubble" + ); +} diff --git a/crates/op-host-desktop/src/design_session_workers.rs b/crates/op-host-desktop/src/design_session_workers.rs index f99f94a1c..96e9f7df8 100644 --- a/crates/op-host-desktop/src/design_session_workers.rs +++ b/crates/op-host-desktop/src/design_session_workers.rs @@ -175,8 +175,17 @@ fn primary_design_message_index(messages: &[ChatMessage]) -> Option { .skip(current_design_turn_start(messages)) .find(|(_, message)| { message.role == ChatRole::Assistant - && message.streaming && message.design_worker_group.is_none() + // The CLI-standard router parks a companion ChatSession beside + // the real DesignSession. At terminal time that chat channel + // can disconnect first and clear `streaming` before the design + // pump drains ValidationDone + RunSummary. Once typed design + // activities exist they are the durable ownership marker; the + // launch-time request marker also covers a fast run whose first + // progress batch arrives after that disconnect. + && (message.streaming + || !message.activities.is_empty() + || message.design_request_json_for_retry.is_some()) }) .map(|(index, _)| index) } @@ -218,7 +227,13 @@ fn current_design_message_indices(messages: &[ChatMessage]) -> Vec { .iter() .enumerate() .skip(current_design_turn_start(messages)) - .filter(|(_, message)| message.role == ChatRole::Assistant && message.streaming) + .filter(|(_, message)| { + message.role == ChatRole::Assistant + && (message.streaming + || message.design_worker_group.is_some() + || !message.activities.is_empty() + || message.design_request_json_for_retry.is_some()) + }) .map(|(index, _)| index) .collect(); if !current.is_empty() { @@ -240,6 +255,36 @@ fn current_design_message_indices(messages: &[ChatMessage]) -> Vec { .collect() } +fn current_owned_design_message_indices(messages: &[ChatMessage]) -> Vec { + // Do not use `design_request_json_for_retry` here: CLI-standard stashes it + // before classifying the turn, so ordinary Chat and Modify bubbles have it + // while their deliberately-unused DesignSession disconnects. + let current: Vec<_> = messages + .iter() + .enumerate() + .skip(current_design_turn_start(messages)) + .filter(|(_, message)| { + message.role == ChatRole::Assistant + && (message.design_worker_group.is_some() || !message.activities.is_empty()) + }) + .map(|(index, _)| index) + .collect(); + if !current.is_empty() { + return current; + } + + messages + .iter() + .enumerate() + .filter(|(_, message)| { + message.role == ChatRole::Assistant + && message.streaming + && (message.design_worker_group.is_some() || !message.activities.is_empty()) + }) + .map(|(index, _)| index) + .collect() +} + pub(super) fn finish_design_success( messages: &mut [ChatMessage], summary: &RunSummary, @@ -413,7 +458,11 @@ pub(super) fn finish_disconnected_design_messages( messages: &mut [ChatMessage], locale: Locale, ) -> bool { - let indices = current_design_message_indices(messages); + // Unlike an explicit Done payload, a bare disconnect can come from the + // unused DesignSession parked beside an ordinary CLI chat request. Require + // durable design ownership here so that channel cannot terminate the real + // plain-chat bubble. + let indices = current_owned_design_message_indices(messages); for &index in &indices { let had_active_activity = messages[index].activities.iter().any(|activity| { matches!(