diff --git a/crates/shell/src/app_chat/mod.rs b/crates/shell/src/app_chat/mod.rs index 0c047a19..60cb686a 100644 --- a/crates/shell/src/app_chat/mod.rs +++ b/crates/shell/src/app_chat/mod.rs @@ -23,7 +23,10 @@ //! independent. //! - **Questions** of the app's conversation (G11: turns the person or the //! app started) are shown and answered here; the system agent's go to the -//! system chat. **Approvals** are the shell's sheets, as everywhere. +//! system chat. While the panel is open, the approvals overlay draws no +//! card for them ([`shown_app`]): its modal card took every press, so the +//! panel's own option buttons answered nothing. **Approvals** are the +//! shell's sheets, as everywhere. //! - **Close** hides the panel. The sharing context stays open with its //! follower, so the conversation keeps up while hidden and a reopen shows //! the person's own rows again: a new context would start an empty @@ -126,6 +129,13 @@ pub fn app() -> Option { with(|p| p.app.clone()) } +/// The app whose panel is open, if one is. Its conversation's questions +/// are this panel's to show and answer, so the approvals overlay draws no +/// card for them ([`crate::approvals::view::card_question`]). +pub fn shown_app() -> Option { + with(|p| if p.open { p.app.as_ref().map(|a| a.id.clone()) } else { None }) +} + /// Whether the keyboard is the panel's. pub fn is_focused() -> bool { with(|p| p.open && p.focused) diff --git a/crates/shell/src/app_chat/tests.rs b/crates/shell/src/app_chat/tests.rs index 4c3716b1..1dfb7fe2 100644 --- a/crates/shell/src/app_chat/tests.rs +++ b/crates/shell/src/app_chat/tests.rs @@ -486,9 +486,11 @@ fn reopening_the_panel_keeps_the_persons_rows_and_the_live_follower() { super::send("what is new?"); assert_eq!(persons_rows(), ["what is new?"]); let context = peers.0.lock().unwrap()[0].1.conversations.lock().unwrap()[0].1.clone(); + assert_eq!(super::shown_app().as_deref(), Some(APP), "open: its questions are the panel's, not the overlay card's"); super::close(); assert!(!super::is_open()); + assert_eq!(super::shown_app(), None, "hidden: the overlay's card asks its questions again"); assert!(context.is_open(), "closing hides the panel; its context stays"); // While hidden, the follower keeps the conversation up to date. let follower = context.follower.lock().unwrap().clone().expect("still followed"); diff --git a/crates/shell/src/approvals/tests.rs b/crates/shell/src/approvals/tests.rs index 7825b914..8157c7b6 100644 --- a/crates/shell/src/approvals/tests.rs +++ b/crates/shell/src/approvals/tests.rs @@ -523,6 +523,41 @@ fn a_batch_is_one_sheet_in_the_system_chat() { assert!(r.answer(sheet.id, &RequestId("b1".into()), Answer::Once, &g, T0).is_err(), "answered once"); } +/// G11: an app's question is shown and answered in its "Ask " panel +/// while that is open. The overlay then draws no card for it: its modal +/// card drew the question a second time and took every press, so the +/// panel's own option buttons answered nothing (an instrument run found +/// it). The card still asks another app's question, and this one once the +/// panel is closed; the system agent's questions are never the overlay's. +#[test] +fn the_overlay_leaves_the_open_panels_questions_to_the_panel() { + use crate::ai_host::app_peers::host_tools::{AgentQuestion, QuestionAnswer, TurnOrigin}; + use crate::questions::Questions; + use super::view::card_question; + let ask = |model: &mut Questions, peer: &str, id: &str, origin: TurnOrigin| { + let mut q = AgentQuestion::parse( + &json!({"question_id": id, "turn_id": format!("turn-{id}"), + "questions": [{"question": "Who should see the summary?", "options": [{"label": "Team"}, {"label": "Everyone"}]}]}), + "_main:api:octosense#peer", + ) + .unwrap(); + q.turn_origin = origin; + model.requested(peer, None, q, QuestionAnswer::new(|_| {})) + }; + let mut model = Questions::default(); + let news = ask(&mut model, "card.os.news", "q1", TurnOrigin::Person); + let calendar = ask(&mut model, "card.os.calendar", "q2", TurnOrigin::App); + ask(&mut model, "card.os.mail", "q3", TurnOrigin::SystemAgent); + let card = |model: &Questions, panel: Option<&str>| card_question(model.open_in_apps(), panel).map(|q| q.id); + assert_eq!(card(&model, None), Some(news), "no panel open: the card asks the oldest app question"); + assert_eq!(card(&model, Some("os.news")), Some(calendar), "Ask News is open: News's question is the panel's, Calendar's is still the card's"); + assert_eq!(card(&model, Some("os.mail")), Some(news), "a panel for another app changes nothing for News"); + let mut news_only = Questions::default(); + let only = ask(&mut news_only, "card.os.news", "q4", TurnOrigin::Person); + assert_eq!(card(&news_only, Some("os.news")), None, "the open panel is the question's one surface: no modal card over it"); + assert_eq!(card(&news_only, None), Some(only), "the panel closed: the card asks it again"); +} + #[test] fn always_for_makes_a_rule_and_answers_the_matching_open_lines() { let (mut r, relay) = router(); diff --git a/crates/shell/src/approvals/view.rs b/crates/shell/src/approvals/view.rs index 96543ea0..0be4188a 100644 --- a/crates/shell/src/approvals/view.rs +++ b/crates/shell/src/approvals/view.rs @@ -17,7 +17,10 @@ //! - **An app agent's question** ([`crate::questions`], the app's //! conversation): who asks, the question and its options; a tap answers //! it (the person's answer, [`crate::questions::PersonAnswer`]). Modal, -//! after any approval sheet. +//! after any approval sheet. Never for the app whose "Ask " panel is +//! open: the panel shows and answers its conversation's questions +//! ([`crate::app_chat`], G11), so each question has one surface +//! ([`card_question`]). //! - **Stop** on an app-conversation sheet or question: denies or declines //! what that app's agent asks and stops the turn running on its shared //! conversation, whoever started it ([`super::stop_agent`]). @@ -135,7 +138,7 @@ struct Frame { more_sheets: usize, consent: Option, everything: Vec, - /// The oldest open question of an app's conversation. + /// The question the card asks ([`card_question`]). question: Option, /// What expired unanswered: approvals, then questions. expired: Vec, @@ -158,13 +161,24 @@ fn frame() -> Frame { } }) .map(|mut f| { - f.question = crate::questions::open_in_apps().into_iter().next(); + f.question = card_question(crate::questions::open_in_apps(), crate::app_chat::shown_app().as_deref()); f.expired_questions = crate::questions::expired_in_apps(); f }) .unwrap_or_default() } +/// The question the overlay's card asks, of the apps' `open` questions +/// (oldest first): the oldest one, except those of the app whose "Ask +/// " panel is open (`panel`). That panel shows and answers its +/// conversation's questions itself (G11); a modal card over it drew the +/// question twice and took every press, so the panel's own buttons +/// answered nothing. +pub(crate) fn card_question(open: Vec, panel: Option<&str>) -> Option { + use crate::questions::Conversation; + open.into_iter().find(|q| !matches!((&q.conversation, panel), (Conversation::App(app), Some(shown)) if app == shown)) +} + #[derive(Script, ScriptHook, Widget)] pub struct ShellApprovals { #[uid] diff --git a/crates/shell/src/host_tools/scenario_tests.rs b/crates/shell/src/host_tools/scenario_tests.rs index 66066e3b..9e4e1125 100644 --- a/crates/shell/src/host_tools/scenario_tests.rs +++ b/crates/shell/src/host_tools/scenario_tests.rs @@ -568,8 +568,13 @@ fn the_system_agent_and_the_person_work_with_one_app_agent_at_once() { // 4. The person answers in the shell's "Ask News" panel (G11: the // question of the app's conversation is shown there, not in the system // chat): their turn ends while the system agent's is still parked on - // its approval. + // its approval. With the panel open it is the question's one surface: + // the approvals overlay draws no card over it. + let card = || crate::approvals::view::card_question(crate::questions::open(&Conversation::App(APP.into())), crate::app_chat::shown_app().as_deref()).map(|q| q.id); + crate::app_chat::close(); + assert_eq!(card(), Some(question.id), "no panel open: the overlay's card asks it"); crate::app_chat::show_for_tests(crate::apps::AgentApp { id: APP.into(), name: LABEL.into(), octos: Vec::new(), manifest: json!({}), native: false }); + assert_eq!(card(), None, "Ask News is open: no card over it, the panel's own buttons answer"); let panel = crate::app_chat::snapshot(); let routed = panel .items diff --git a/crates/shell/src/questions/mod.rs b/crates/shell/src/questions/mod.rs index 96dd295d..c1d2414d 100644 --- a/crates/shell/src/questions/mod.rs +++ b/crates/shell/src/questions/mod.rs @@ -26,10 +26,12 @@ //! when the peer's `generic_tools` lists it. //! //! **Consumers.** The system chat subscribes and shows its questions in -//! its conversation (`crate::system_chat`); the shell's app-conversation -//! surface (the approvals overlay, `crate::approvals::view`) shows the -//! app's. Notification and glance cards can subscribe the same way; none -//! of them owns a question, and none renders anything here. +//! its conversation (`crate::system_chat`). An app's are shown in its "Ask +//! " panel (`crate::app_chat`) while that is open, and otherwise on +//! the shell's app-conversation surface (the approvals overlay's card, +//! `crate::approvals::view`): one surface at a time. Notification and +//! glance cards can subscribe the same way; none of them owns a question, +//! and none renders anything here. //! //! **Only the person answers, only through the shell.** The one answer //! path is [`answer`], which needs a [`PersonAnswer`]: made only by the