Glance: the panel's cards run their taps and in-card chat; chat threads keep Mail's account

crates/l0-chat/README.md

@@ -20,7 +20,7 @@ Octoscript's L0 checker admits the card. The rest is the host's, and is here
 | **The transcript is the host's** | `seed` puts the host's own transcript under every `sys.chat` source's name, replacing whatever the publisher put in its `data`: no forged `model` entry, no other app's thread. |
 | **Roles** | Entries are `{id, role, text, at}`, `role` one of `user`, `model`, `host`. A card's one write is `append`, recorded as a `user` entry only when its payload is what the person typed (`ValueOrigin::UserInput`). Only the host's `ChatStore::append_reply` writes `model` (the agent's answer, drawn AI-written) or `host` (a notice). A payload that looks like `{"role":"model",…}` is just the text of a `user` entry. |
 | **Limits** | `TEXT_MAX_BYTES` 4 KiB per message after trimming, control characters dropped; one message per thread every `MIN_INTERVAL_MS` (2 s) and none while the agent is still answering; the last `RETENTION` (200) entries kept; a reply cut at `REPLY_MAX_BYTES` (16 KiB); app and thread ids at most `ID_MAX` (64), threads `[A-Za-z0-9_-]`. |
-| **Storage** | `ChatStore::with_folder` keeps one JSON file per (app, thread) in the folder the host names; `ChatStore::in_memory` for tests. The shell names the app's account folder (ADR 0004 §11): `apps/<app>/accounts/<account>/chat/<thread>.json`, owner-only, for the account the app's agent acts for (`device` without one), so switching accounts switches transcripts. AppCard keeps its chats in `apps/appcard/accounts/device/chat/`. |
+| **Storage** | `ChatStore::with_folder` keeps one JSON file per (app, thread) in the folder the host names; `ChatStore::in_memory` for tests. The shell names the app's account folder (ADR 0004 §11): `apps/<app>/accounts/<account>/chat/<thread>.json`, owner-only, for the account the app's agent acts for (`device` without one), so switching accounts switches transcripts; a reply is kept in the thread its message went to, even if the account switched while the agent answered. AppCard keeps its chats in `apps/appcard/accounts/device/chat/`. |
 | **Stale** | Every change bumps `ChatStore::generation` and calls the host's change hook (`set_on_change`), so a surface re-seeds and re-lowers the card when the reply arrives. |
 
 ## Who answers: `Responder`

crates/l0-chat/src/lib.rs

@@ -35,7 +35,9 @@
 //! - **Storage.** Threads are kept per app and thread, in the folder the
 //!   host's [`ChatStore::with_folder`] names for the app (the shell: the
 //!   app's account folder under ADR 0004's layout, `apps/<app>/accounts/
-//!   <account>/chat/<thread>.json`), or in memory.
+//!   <account>/chat/<thread>.json`), or in memory. A reply is kept in the
+//!   thread its message went to, even when the host names another folder
+//!   by then (another account signed in while the agent answered).
 //! - **Stale.** Every change bumps [`ChatStore::generation`] and calls the
 //!   host's change hook, so a surface re-seeds and re-lowers the card: the
 //!   write marks the source stale (§5.9) and the reply arrives later.
@@ -255,8 +257,8 @@ impl ChatStore {
             .unwrap_or_default()
     }
 
-    fn save(&self, app: &str, thread: &str, t: &Thread) {
-        let Some(path) = self.file(app, thread) else {
+    fn save(&self, app: &str, thread: &str, file: &Option<PathBuf>, t: &Thread) {
+        let Some(path) = file else {
             return;
         };
         let disk = json!({"schema": SCHEMA, "entries": t.entries, "next": t.next});
@@ -267,31 +269,48 @@ impl ChatStore {
                 use std::os::unix::fs::PermissionsExt;
                 std::fs::set_permissions(&tmp, std::fs::Permissions::from_mode(0o600))?;
             }
-            std::fs::rename(&tmp, &path)
+            std::fs::rename(&tmp, path)
         });
         if let Err(e) = written {
             eprintln!("l0 chat: could not keep {app}/{thread}: {e}");
         }
     }
 
-    fn with_thread<R>(&self, app: &str, thread: &str, f: impl FnOnce(&mut Thread) -> R) -> R {
+    /// `f` on the thread in the folder the host names for the app now.
+    /// Keyed by the file too: when the host's folder for the app changes
+    /// (another account is active), the thread is that folder's.
+    fn with_thread<R>(
+        &self,
+        app: &str,
+        thread: &str,
+        f: impl FnOnce(&mut Thread, &Option<PathBuf>) -> R,
+    ) -> R {
+        self.with_thread_at(app, thread, self.file(app, thread), f)
+    }
+
+    /// `f` on the thread kept in `file` (`None`: in memory), and that file.
+    fn with_thread_at<R>(
+        &self,
+        app: &str,
+        thread: &str,
+        file: Option<PathBuf>,
+        f: impl FnOnce(&mut Thread, &Option<PathBuf>) -> R,
+    ) -> R {
         let mut threads = self.threads.lock().unwrap_or_else(|e| e.into_inner());
-        // Keyed by the file too: when the host's folder for the app changes
-        // (another account is active), the thread is that folder's.
-        let key = (app.to_string(), thread.to_string(), self.file(app, thread));
+        let key = (app.to_string(), thread.to_string(), file);
         if !threads.contains_key(&key) {
             let loaded = self.load(&key.2);
             threads.insert(key.clone(), loaded);
         }
-        f(threads.get_mut(&key).expect("just inserted"))
+        f(threads.get_mut(&key).expect("just inserted"), &key.2)
     }
 
     /// The thread's entries, oldest first.
     pub fn entries(&self, app: &str, thread: &str) -> Vec<Entry> {
         if !valid_app(app) || !valid_thread(thread) {
             return Vec::new();
         }
-        self.with_thread(app, thread, |t| t.entries.clone())
+        self.with_thread(app, thread, |t, _| t.entries.clone())
     }
 
     /// `sys.chat`'s answer for one thread: `{status, count, entries}`;
@@ -300,7 +319,7 @@ impl ChatStore {
         if !valid_app(app) || !valid_thread(thread) {
             return unavailable();
         }
-        self.with_thread(app, thread, |t| {
+        self.with_thread(app, thread, |t, _| {
             json!({
                 "status": if t.answering { "answering" } else { "ready" },
                 "count": t.entries.len(),
@@ -317,6 +336,18 @@ impl ChatStore {
         thread: &str,
         text: &str,
         now: u64,
+    ) -> Result<Entry, String> {
+        self.append_user_at(app, thread, self.file(app, thread), text, now)
+    }
+
+    /// [`Self::append_user`] to the thread kept in `file`.
+    fn append_user_at(
+        &self,
+        app: &str,
+        thread: &str,
+        file: Option<PathBuf>,
+        text: &str,
+        now: u64,
     ) -> Result<Entry, String> {
         if !valid_app(app) || !valid_thread(thread) {
             return Err(format!("not a conversation: {app}/{thread}"));
@@ -331,7 +362,7 @@ impl ChatStore {
                 text.len()
             ));
         }
-        let entry = self.with_thread(app, thread, |t| {
+        let entry = self.with_thread_at(app, thread, file, |t, file| {
             if t.answering {
                 return Err("the agent is still answering the last message".to_string());
             }
@@ -346,7 +377,7 @@ impl ChatStore {
             let entry = push(t, Role::User, text, now);
             t.last_user_ms = Some(now);
             t.answering = true;
-            self.save(app, thread, t);
+            self.save(app, thread, file, t);
             Ok(entry)
         })?;
         self.changed();
@@ -356,6 +387,18 @@ impl ChatStore {
     /// The host appends the agent's reply (`model`) or a notice (`host`),
     /// and the thread takes messages again.
     pub fn append_reply(&self, app: &str, thread: &str, reply: Reply, now: u64) -> Option<Entry> {
+        self.append_reply_at(app, thread, self.file(app, thread), reply, now)
+    }
+
+    /// [`Self::append_reply`] to the thread kept in `file`.
+    fn append_reply_at(
+        &self,
+        app: &str,
+        thread: &str,
+        file: Option<PathBuf>,
+        reply: Reply,
+        now: u64,
+    ) -> Option<Entry> {
         if !valid_app(app) || !valid_thread(thread) {
             return None;
         }
@@ -367,10 +410,10 @@ impl ChatStore {
             Reply::Notice(text) => (Role::Host, text),
         };
         let text = cut(text.trim(), REPLY_MAX_BYTES);
-        let entry = self.with_thread(app, thread, |t| {
+        let entry = self.with_thread_at(app, thread, file, |t, file| {
             let entry = push(t, role, text, now);
             t.answering = false;
-            self.save(app, thread, t);
+            self.save(app, thread, file, t);
             entry
         });
         self.changed();
@@ -389,14 +432,14 @@ impl ChatStore {
         if !valid_app(app) || !valid_thread(thread) {
             return false;
         }
-        let seeded = self.with_thread(app, thread, |t| {
+        let seeded = self.with_thread(app, thread, |t, file| {
             if !t.entries.is_empty() {
                 return false;
             }
             for (role, text) in entries {
                 push(t, *role, text.to_string(), now);
             }
-            self.save(app, thread, t);
+            self.save(app, thread, file, t);
             true
         });
         if seeded {
@@ -618,8 +661,12 @@ pub fn perform(
         ));
     }
     let thread = thread_of(&source, state, data).ok_or("sys.chat: no valid thread")?;
-    let entry = chat.append_user(&app, &thread, &write.value, now)?;
-    let history = chat.entries(&app, &thread);
+    // The reply goes where the message went: the thread in the folder the
+    // host names now, even when it names another (another account signed
+    // in) before the agent answers.
+    let file = chat.file(&app, &thread);
+    let entry = chat.append_user_at(&app, &thread, file.clone(), &write.value, now)?;
+    let history = chat.with_thread_at(&app, &thread, file.clone(), |t, _| t.entries.clone());
     let store = chat.clone();
     let (a, t) = (app.clone(), thread.clone());
     responder.respond(
@@ -630,7 +677,7 @@ pub fn perform(
             history,
         },
         Box::new(move |reply| {
-            store.append_reply(&a, &t, reply, now_ms());
+            store.append_reply_at(&a, &t, file, reply, now_ms());
         }),
     );
     Ok(entry)

crates/l0-chat/src/tests.rs

@@ -498,3 +498,75 @@ fn threads_are_kept_in_the_apps_folder() {
             .is_empty()
     );
 }
+
+/// A thread is the folder's the host names for the app now: another
+/// account signed in, another thread. A reply goes to the thread its
+/// message went to, even when the account switched while the agent
+/// answered: never into the new account's thread, and the first thread
+/// takes messages again.
+#[test]
+fn a_reply_stays_with_the_account_its_message_went_to() {
+    struct Later(Mutex<Option<Done>>);
+    impl Responder for Later {
+        fn respond(&self, _: Request, done: Done) {
+            *self.0.lock().unwrap() = Some(done);
+        }
+    }
+    let root = std::env::temp_dir().join(format!("l0-chat-accounts-{}", std::process::id()));
+    let _ = std::fs::remove_dir_all(&root);
+    let account = Arc::new(Mutex::new("a"));
+    let folder = {
+        let (root, account) = (root.clone(), account.clone());
+        move |app: &str| {
+            let dir = root.join(app).join(*account.lock().unwrap()).join("chat");
+            std::fs::create_dir_all(&dir).ok()?;
+            Some(dir)
+        }
+    };
+    let chat = Arc::new(ChatStore::with_folder(Box::new(folder)));
+    let later = Later(Mutex::new(None));
+    let (write, origin, data) = send(&chat, "os.news", CARD, "asked as a");
+    perform(
+        &chat,
+        &later,
+        "os.news",
+        CARD,
+        &InstanceStore::default(),
+        &data,
+        &write,
+        origin,
+        0,
+    )
+    .unwrap();
+    // Another account signs in while the agent answers.
+    *account.lock().unwrap() = "b";
+    assert!(chat.entries("os.news", "main").is_empty(), "b's own thread");
+    let done = later.0.lock().unwrap().take().unwrap();
+    done(Reply::Model("for a".into()));
+    assert!(
+        chat.entries("os.news", "main").is_empty(),
+        "a's reply is not b's"
+    );
+    assert!(!root.join("os.news/b/chat/main.json").exists());
+    *account.lock().unwrap() = "a";
+    assert_eq!(
+        roles(&chat, "os.news", "main"),
+        [
+            (Role::User, "asked as a".to_string()),
+            (Role::Model, "for a".to_string())
+        ]
+    );
+    assert_eq!(
+        chat.answer("os.news", "main")["status"],
+        "ready",
+        "a's thread takes messages again"
+    );
+    let kept = ChatStore::with_folder(Box::new(move |app: &str| {
+        Some(root.join(app).join("a").join("chat"))
+    }));
+    assert_eq!(
+        kept.entries("os.news", "main").len(),
+        2,
+        "kept on disk as a's"
+    );
+}

crates/shell/src/app_storage/lifecycle.rs

@@ -171,13 +171,20 @@ pub fn account_changed(storage: &Arc<Storage>, service_app: &str, previous: Opti
 /// opened): record its manifest's storage block, so its agent acts for the
 /// right account and its suspension is keyed right; then `prepare`.
 pub fn prepare_agent_with<R>(storage: &Storage, root: &Path, app_id: &str, prepare: impl FnOnce(&Storage) -> R) -> R {
+    record_manifest_spec(storage, root, app_id);
+    prepare(storage)
+}
+
+/// Record the storage block of the script app `app_id`'s manifest, when
+/// one is on disk ([`script_manifest`]); a block the host refuses is
+/// logged, and the default holds.
+fn record_manifest_spec(storage: &Storage, root: &Path, app_id: &str) {
     if let Some(manifest) = script_manifest(root, app_id) {
         match StorageSpec::from_manifest(&manifest, AppKind::Script) {
             Ok(spec) => storage.set_spec(app_id, spec),
             Err(e) => makepad_widgets::log!("app storage: {app_id}: {e}"),
         }
     }
-    prepare(storage)
 }
 
 /// The account a contained app's agent acts for (`contained::set_account_of`).
@@ -192,7 +199,17 @@ pub fn contained_account(app: &str) -> Option<String> {
 /// (Mail), its active account (the one the person signed in to last, from
 /// Mail's host service), or none yet. One agent is live per app at a time,
 /// bound to that account (ADR 0004 §11).
+///
+/// Whether the app keeps accounts is its manifest's storage block. Nothing
+/// may have recorded it yet in this run: the account is also asked before
+/// the app was opened or its agent prepared (a glance card's chat thread at
+/// startup, glance_chat.rs), so the block is read from the manifest then,
+/// as [`prepare_agent_with`] does. The default, no accounts, would answer
+/// the device for a signed-in Mail.
 pub fn contained_account_in(storage: &Storage, app: &str) -> Option<String> {
+    if !storage.has_spec(app) {
+        record_manifest_spec(storage, storage.layout().apps_root(), app);
+    }
     if !storage.spec(app).accounts {
         return Some(crate::ai_host::contained::ACCOUNT.to_owned());
     }

crates/shell/src/app_storage/lifecycle_tests.rs

@@ -265,6 +265,28 @@ fn should_bind_a_contained_agent_to_the_active_account_when_the_app_keeps_accoun
     assert_eq!(contained_account_in(&host, "os.mail").as_deref(), Some("late"));
 }
 
+/// The account is also asked before anything recorded the app's storage
+/// block in this run (a glance card's chat thread at startup, before Mail
+/// was opened or its agent prepared): the block is read from the manifest
+/// then and recorded, so a signed-in Mail acts for its account, never the
+/// device.
+#[cfg(any(feature = "app-hub", native_mobile))]
+#[test]
+fn should_read_the_manifests_block_when_the_account_is_asked_first() {
+    let home = Scratch::new("account-first");
+    let host = storage(&home.0);
+    let root = host.layout().apps_root().to_path_buf();
+    write_json(&root.join(".system/os.mail/0001/manifest.json"), &json!({"id": "os.mail", "storage": {"accounts": true}}));
+    write_json(&root.join(".host/mail/accounts.json"), &json!([{"id": "ana@example.org", "apps": ["os.mail"], "signed_in": 10}]));
+    assert!(!host.has_spec("os.mail"));
+    assert_eq!(contained_account_in(&host, "os.mail").as_deref(), Some("ana@example.org"));
+    assert!(host.has_spec("os.mail") && host.spec("os.mail").accounts, "recorded from the manifest");
+    // No manifest on disk yet (a system app not unpacked): the device, and
+    // nothing recorded, so a later ask reads it once it is there.
+    assert_eq!(contained_account_in(&host, "os.notes").as_deref(), Some("device"));
+    assert!(!host.has_spec("os.notes"));
+}
+
 // ---- script manifests, install and uninstall -----------------------------
 
 #[test]

crates/shell/src/app_storage/mod.rs

@@ -434,6 +434,12 @@ impl Storage {
         self.state().specs.get(app_id).cloned().unwrap_or_default()
     }
 
+    /// Whether `app_id`'s storage block was recorded yet ([`Self::spec`]
+    /// answers the default until then).
+    pub fn has_spec(&self, app_id: &str) -> bool {
+        self.state().specs.contains_key(app_id)
+    }
+
     /// Lay out `app_id`'s jail (`accounts/`, `common/`, `cache/`) and its
     /// secrets directory, and return the handle a module is offered.
     pub fn open(self: &Arc<Self>, app_id: &str) -> Result<Arc<dyn AppStorage>, StorageError> {