diff --git a/crates/shell/src/app_chat/mod.rs b/crates/shell/src/app_chat/mod.rs index 60cb686a..6085c006 100644 --- a/crates/shell/src/app_chat/mod.rs +++ b/crates/shell/src/app_chat/mod.rs @@ -50,6 +50,8 @@ use crate::system_chat::model::{ChatModel, Item, Phase}; use model::Conversation; /// The panel's client instance on the app's peer (the same on every open). +/// Internal: the sheets and question cards say "for you" for it +/// (`approvals::sheet::persons_surface`). pub const INSTANCE: &str = "shell-ask"; /// The id prefix of the app's questions in the panel. diff --git a/crates/shell/src/approvals/sheet.rs b/crates/shell/src/approvals/sheet.rs index 59a2eb57..4cbddc96 100644 --- a/crates/shell/src/approvals/sheet.rs +++ b/crates/shell/src/approvals/sheet.rs @@ -105,10 +105,21 @@ pub fn app_label(app: &str) -> String { } } -/// "Calendar's agent", "The system agent", "Rinx's agent for weather". +/// Whether a request context's `client` is one of the shell's own surfaces +/// where the person talks to an app's agent: the "Ask " panel +/// ([`crate::app_chat::INSTANCE`]) or a card's in-card chat +/// ([`crate::glance_chat::INSTANCE`]). Such a turn is the person's, and +/// the person reads "for you", never the shell's internal instance id. +pub fn persons_surface(client: &str) -> bool { + client == crate::app_chat::INSTANCE || client == crate::glance_chat::INSTANCE +} + +/// "Calendar's agent", "The system agent", "Rinx's agent for weather", +/// "Calendar's agent for you" (the person's own surface, [`persons_surface`]). pub fn caller_label(owning_app: &str, caller: &Caller) -> String { match caller { Caller::OwnAgent { client: None } => format!("{}'s agent", app_label(owning_app)), + Caller::OwnAgent { client: Some(c) } if persons_surface(c) => format!("{}'s agent for you", app_label(owning_app)), Caller::OwnAgent { client: Some(c) } => format!("{}'s agent for {c}", app_label(owning_app)), Caller::AppAgent { app } => format!("{}'s agent", app_label(app)), Caller::SystemAgent => "The system agent".into(), diff --git a/crates/shell/src/approvals/tests.rs b/crates/shell/src/approvals/tests.rs index 8157c7b6..56aa4b66 100644 --- a/crates/shell/src/approvals/tests.rs +++ b/crates/shell/src/approvals/tests.rs @@ -494,6 +494,30 @@ fn sheet_model_shows_app_tool_caller_and_redacted_args() { assert!(labels.iter().any(|l| l.starts_with("Everything Mail asks"))); } +/// A turn from the person's own surface (the "Ask " panel, a card's +/// chat) is the person's: the sheet line and its toast say "for you". The +/// shell's internal instance id reached the person ("Asked by Calendar's +/// agent for shell-ask", the toast "Calendar's agent for shell-ask asks"). +/// Another client keeps its own name (a Rinx mini app). +#[test] +fn the_persons_own_surface_reads_for_you_never_its_instance_id() { + use super::sheet::caller_label; + for client in [crate::app_chat::INSTANCE, crate::glance_chat::INSTANCE] { + assert_eq!(caller_label("os.calendar", &Caller::OwnAgent { client: Some(client.into()) }), "Calendar's agent for you", "{client}"); + } + assert_eq!(caller_label("rinx", &Caller::OwnAgent { client: Some("weather".into()) }), "Rinx's agent for weather"); + assert_eq!(caller_label("os.calendar", &Caller::OwnAgent { client: None }), "Calendar's agent"); + let (mut r, _) = router(); + let caller = Caller::OwnAgent { client: Some(crate::app_chat::INSTANCE.into()) }; + let request = make_request("os.calendar", ToolSpec::host("calendar.remove_event"), json!({"id": "evt-1"}), caller, ctx("ask-1", Trigger::Person), T0, 0); + let Route::Sheet(_) = r.request(request, T0) else { panic!() }; + assert_eq!(r.front_sheet().unwrap().lines[0].caller, "Calendar's agent for you", "the sheet's \"Asked by\" line"); + let notices = r.take_notices(); + let toast = notices.iter().find(|n| n.title == "Needs you: Calendar \u{00b7} calendar.remove_event").expect("the sheet's toast"); + assert_eq!(toast.body, "Calendar's agent for you asks. Open the sheet to approve or deny."); + assert!(notices.iter().all(|n| !format!("{} {}", n.title, n.body).contains(crate::app_chat::INSTANCE)), "{notices:?}"); +} + #[test] fn a_batch_is_one_sheet_in_the_system_chat() { let (mut r, relay) = router(); diff --git a/crates/shell/src/glance_chat.rs b/crates/shell/src/glance_chat.rs index 87145441..3538e905 100644 --- a/crates/shell/src/glance_chat.rs +++ b/crates/shell/src/glance_chat.rs @@ -118,8 +118,9 @@ impl Responder for HostResponder { /// first. pub struct AgentResponder; -/// The instance the card chat's conversation is opened under. -const INSTANCE: &str = "card-chat"; +/// The instance the card chat's conversation is opened under (internal: +/// the person reads "for you", `approvals::sheet::persons_surface`). +pub const INSTANCE: &str = "card-chat"; impl Responder for AgentResponder { fn respond(&self, request: Request, done: Done) { diff --git a/crates/shell/src/questions/mod.rs b/crates/shell/src/questions/mod.rs index c1d2414d..504af1b3 100644 --- a/crates/shell/src/questions/mod.rs +++ b/crates/shell/src/questions/mod.rs @@ -128,11 +128,16 @@ impl Request { pub fn answer_count(&self) -> usize { self.items.len().max(1) } - /// "Rinx's agent asks", for a surface's heading. + /// "Rinx's agent asks", for a surface's heading: "(for the assistant)" + /// in the system agent's lane, "(for you)" from the person's own surface + /// (the "Ask " panel, a card's chat: + /// [`crate::approvals::sheet::persons_surface`]), else the client's name + /// ("(weather)", a Rinx mini app). Never the shell's internal instance id. pub fn asked_by(&self) -> String { let app = crate::approvals::sheet::app_label(&self.app); match (&self.origin, &self.client) { (Origin::SystemAgent, _) => format!("{app}'s agent asks (for the assistant)"), + (_, Some(client)) if crate::approvals::sheet::persons_surface(client) => format!("{app}'s agent asks (for you)"), (_, Some(client)) => format!("{app}'s agent asks ({client})"), _ => format!("{app}'s agent asks"), } diff --git a/crates/shell/src/questions/tests.rs b/crates/shell/src/questions/tests.rs index c525fbac..e78164ac 100644 --- a/crates/shell/src/questions/tests.rs +++ b/crates/shell/src/questions/tests.rs @@ -90,6 +90,28 @@ fn a_question_follows_its_turns_origin_not_its_session() { assert!(model.get(system).unwrap().origin_reported); } +/// Who asks, as every surface heads a question: the person's own surface +/// (the "Ask " panel, a card's chat) reads "for you", never the +/// shell's internal instance id ("News's agent asks (shell-ask)" reached +/// the person); the system agent's lane "for the assistant"; another +/// client (a Rinx mini app) keeps its name. +#[test] +fn who_asks_names_the_person_never_the_shells_instance_id() { + let mut model = Questions::default(); + let mut ask = |peer: &str, client: Option<&str>, origin: CallOrigin| { + let mut q = question("q1", "t1", origin, client.map(|_| "ctx1")); + q.client = client.map(str::to_string); + let (answer, _) = answer_handle(); + let id = model.requested(peer, None, q, answer); + model.get(id).unwrap().asked_by() + }; + assert_eq!(ask("card.os.news", Some(crate::app_chat::INSTANCE), CallOrigin::Context), "News's agent asks (for you)"); + assert_eq!(ask("card.os.mail", Some(crate::glance_chat::INSTANCE), CallOrigin::Context), "Mail's agent asks (for you)"); + assert_eq!(ask("card.os.news", None, CallOrigin::PeerInput), "News's agent asks (for the assistant)"); + assert_eq!(ask("rinx", Some("weather"), CallOrigin::Context), "Rinx's agent asks (weather)"); + assert_eq!(ask("rinx", None, CallOrigin::PeerOwn), "Rinx's agent asks"); +} + #[test] fn the_persons_answer_is_sent_once_and_a_closed_question_takes_none() { let mut model = Questions::default();