Approvals: a sheet's "Needs you" notification goes with its sheet line

crates/shell/src/approvals/mod.rs

@@ -21,7 +21,8 @@
 //! [`contacts::CONTACTS_FILE`] and [`audit::AUDIT_FILE`], all owner-only.
 //!
 //! The shell calls [`init`] at startup, [`tick`] once a second (and shows
-//! [`take_notices`] as notifications), and gives pointer events to
+//! [`take_notices`] as notifications, withdrawing a request's own once it
+//! no longer waits: [`RequestNotices`]), and gives pointer events to
 //! [`pointer`] before anything else while a sheet or the Settings page is
 //! up. The relay calls [`approval_requested`] and installs itself with
 //! [`set_relay`]; an app module registers its own confirmation sheet with
@@ -296,6 +297,49 @@ pub fn dismiss_expired(id: &RequestId) {
 pub fn take_notices() -> Vec<Notice> {
     with(|a| a.router.take_notices()).unwrap_or_default()
 }
+
+/// Whether `id` still waits for its answer (on its sheet, or on the
+/// owning app's).
+pub fn is_pending(id: &RequestId) -> bool {
+    with(|a| a.router.is_pending(id)).unwrap_or(false)
+}
+
+/// Where the shell showed one notice: its desktop toast and its phone shade
+/// notification.
+#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
+pub struct Shown {
+    pub toast: Option<u64>,
+    pub shade: Option<u64>,
+}
+
+/// The notifications of the notices that ask the person to answer a request
+/// ([`Notice::request`]: "Needs you: … Open the sheet to approve or
+/// deny."), by that request. The shell withdraws each once its request is
+/// no longer pending, together with its sheet line: after Stop withdrew
+/// the sheet, its toast still said to open it.
+#[derive(Debug, Default)]
+pub struct RequestNotices(Vec<(RequestId, Shown)>);
+
+impl RequestNotices {
+    pub fn record(&mut self, request: RequestId, shown: Shown) {
+        self.0.push((request, shown));
+    }
+
+    /// The notifications of the requests `pending` no longer holds, to
+    /// withdraw now; forgotten here.
+    pub fn withdrawn(&mut self, pending: impl Fn(&RequestId) -> bool) -> Vec<(RequestId, Shown)> {
+        let mut gone = Vec::new();
+        self.0.retain(|(id, shown)| {
+            let keep = pending(id);
+            if !keep {
+                gone.push((id.clone(), *shown));
+            }
+            keep
+        });
+        gone
+    }
+}
+
 pub fn generation() -> u64 {
     with(|a| a.generation()).unwrap_or(0)
 }

crates/shell/src/approvals/router.rs

@@ -121,6 +121,11 @@ const EXPIRED_KEPT: usize = 16;
 pub struct Notice {
     pub title: String,
     pub body: String,
+    /// The request this notice asks the person to answer ("Needs you: …
+    /// Open the sheet"): its notification goes once the request is no
+    /// longer pending, with its sheet line (answered, stopped, withdrawn,
+    /// expired). `None`: the notice stands on its own.
+    pub request: Option<RequestId>,
 }
 
 pub struct Router {
@@ -276,7 +281,7 @@ impl Router {
         std::mem::take(&mut self.notices)
     }
     fn notice(&mut self, title: impl Into<String>, body: impl Into<String>) {
-        self.notices.push(Notice { title: title.into(), body: body.into() });
+        self.notices.push(Notice { title: title.into(), body: body.into(), request: None });
     }
 
     /// Route one request. The decision may be given before this returns.
@@ -361,7 +366,12 @@ impl Router {
     fn surface(&mut self, req: &Request, surfaced: Surfaced, now: u64) -> u64 {
         let line = Line::for_request(req, surfaced, &self.contacts);
         let batch = req.context.batch.clone();
-        self.notice(format!("Needs you: {}", line.heading()), format!("{} asks. Open the sheet to approve or deny.", line.caller));
+        // Withdrawn with the sheet line: it points at the sheet.
+        self.notices.push(Notice {
+            title: format!("Needs you: {}", line.heading()),
+            body: format!("{} asks. Open the sheet to approve or deny.", line.caller),
+            request: Some(req.id.clone()),
+        });
         self.changed();
         if let Some(b) = &batch {
             if let Some(sheet) = self.sheets.iter_mut().find(|s| s.batch_id() == Some(&b.id) && !s.done()) {

crates/shell/src/approvals/tests.rs

@@ -719,6 +719,52 @@ fn stop_denies_what_the_stopped_agent_asks() {
     assert_eq!(r.sheets().len(), 1);
 }
 
+/// The sheet's "Needs you … Open the sheet" notice names its request, and
+/// the shell withdraws that notification once the request no longer waits,
+/// with its sheet line: after the person's Stop (an instrument run found
+/// the toast still up, pointing at a sheet that was gone), an answer on
+/// the sheet, the turn's own end, or the deadline. Other notices stand on
+/// their own.
+#[test]
+fn a_sheets_notification_is_withdrawn_with_its_line() {
+    let (mut r, _) = router();
+    r.sheet_expiry_s = 600;
+    let own = |id: &str| {
+        let mut q = send(id, json!({"to": "eve@example.org"}));
+        q.caller = Caller::OwnAgent { client: None };
+        q
+    };
+    let Route::Sheet(_) = r.request(send("stopped", json!({"to": "eve@example.org"})), T0) else { panic!() };
+    let Route::Sheet(answered_on) = r.request(own("answered"), T0) else { panic!() };
+    let Route::Sheet(_) = r.request(own("withdrawn"), T0) else { panic!() };
+    let mut late = own("expired");
+    late.received = T0 + 60;
+    let Route::Sheet(_) = r.request(late, T0 + 60) else { panic!() };
+    let mut shown = super::RequestNotices::default();
+    let notices = r.take_notices();
+    assert_eq!(notices.len(), 4, "{notices:?}");
+    for (n, notice) in notices.iter().enumerate() {
+        assert!(notice.title.starts_with("Needs you: ") && notice.body.ends_with("Open the sheet to approve or deny."), "{notice:?}");
+        let request = notice.request.clone().expect("a sheet's notice names its request");
+        shown.record(request, super::Shown { toast: Some(n as u64 + 1), shade: Some(n as u64 + 11) });
+    }
+    let mut withdrawn = |r: &Router| -> Vec<String> { shown.withdrawn(|id| r.is_pending(id)).into_iter().map(|(id, _)| id.0).collect() };
+    assert!(withdrawn(&r).is_empty(), "all four still wait on their sheets");
+    r.stop_agent("calendar", T0 + 1);
+    assert_eq!(withdrawn(&r), ["stopped"], "Stop withdrew the line: its toast goes too");
+    r.answer(answered_on, &RequestId("answered".into()), Answer::Deny, &ApprovalGesture::sheet_tap(), T0 + 2).unwrap();
+    assert_eq!(withdrawn(&r), ["answered"]);
+    assert!(r.withdraw(&RequestId("withdrawn".into()), "the turn ended", T0 + 3));
+    assert_eq!(withdrawn(&r), ["withdrawn"]);
+    r.tick(T0 + 600);
+    assert!(withdrawn(&r).is_empty(), "the last one has a minute left");
+    r.tick(T0 + 660);
+    assert_eq!(withdrawn(&r), ["expired"]);
+    let expired = r.take_notices();
+    assert!(expired.iter().any(|n| n.title.starts_with("Expired: ") && n.request.is_none()), "the expiry notice stands on its own: {expired:?}");
+    assert!(withdrawn(&r).is_empty(), "each is withdrawn once");
+}
+
 // ---------------------------------------------------------------- confirm: app
 
 #[derive(Clone, Default)]

crates/shell/src/lib.rs

@@ -79,6 +79,8 @@ pub mod glance_notice;
 pub mod glance_panel;
 pub mod glance_sheet;
 use glance::NoteTargets as GlanceNoteTargets;
+// The App derive takes a plain type name for a field.
+use approvals::RequestNotices as ApprovalNotices;
 pub mod system_chat;
 pub use octosense_ai_host as ai_host;
 
@@ -437,6 +439,10 @@ pub struct App {
     /// The approvals' generation last drawn (approvals/).
     #[rust]
     pub approvals_generation: u64,
+    /// The notifications of "Needs you" notices, withdrawn with their
+    /// requests' sheet lines (approvals/).
+    #[rust]
+    pub approval_notices: ApprovalNotices,
     /// The system chat's generation the surfaces last drew (system_chat/).
     #[rust]
     pub system_chat_generation: u64,
@@ -4134,7 +4140,16 @@ impl App {
             self.on_bus_route(cx, route);
         }
         for n in approvals::take_notices() {
-            self.notify(cx, &n.title, &n.body);
+            let shown = self.notify_shown(cx, &n.title, &n.body);
+            if let Some(request) = n.request {
+                self.approval_notices.record(request, shown);
+            }
+        }
+        // "Needs you … Open the sheet" goes with its sheet line (answered,
+        // stopped, withdrawn, expired): it pointed at a sheet that was gone.
+        for (request, shown) in self.approval_notices.withdrawn(approvals::is_pending) {
+            log!("approvals: {request} no longer waits; its notification is withdrawn");
+            self.withdraw_notification(cx, shown);
         }
         let generation = approvals::generation();
         if generation != self.approvals_generation {
@@ -4146,15 +4161,32 @@ impl App {
     /// The in-process notification API — `WmRequest::Notify{title, body}`
     /// lands here.
     pub fn notify(&mut self, cx: &mut Cx, title: &str, body: &str) {
+        self.notify_shown(cx, title, body);
+    }
+
+    /// [`Self::notify`], saying where it was shown (the desktop toast, the
+    /// phone shade's notification), so it can be withdrawn.
+    fn notify_shown(&mut self, cx: &mut Cx, title: &str, body: &str) -> approvals::Shown {
         let notes = self.ui.widget(cx, ids!(shell_notes));
-        {
+        let toast = notes.borrow_mut::<shell::notifications::ShellNotifications>().map(|mut n| n.notify(cx, title, body));
+        let now = cx.seconds_since_app_start();
+        let shade = self.state.as_mut().map(|state| state.phone.shade.post("wm", title, body, now, Vec::new()));
+        self.redraw_all(cx);
+        approvals::Shown { toast, shade }
+    }
+
+    /// Take back what [`Self::notify_shown`] showed (gone already: nothing).
+    fn withdraw_notification(&mut self, cx: &mut Cx, shown: approvals::Shown) {
+        if let Some(id) = shown.toast {
+            let notes = self.ui.widget(cx, ids!(shell_notes));
             let mut borrowed = notes.borrow_mut::<shell::notifications::ShellNotifications>();
             if let Some(n) = borrowed.as_mut() {
-                n.notify(cx, title, body);
+                n.dismiss(cx, id);
             }
         }
-        let now = cx.seconds_since_app_start();
-        if let Some(state) = self.state.as_mut() { state.phone.shade.post("wm", title, body, now, Vec::new()); }
+        if let (Some(id), Some(state)) = (shown.shade, self.state.as_mut()) {
+            state.phone.shade.dismiss(id);
+        }
         self.redraw_all(cx);
     }