bread-launcher: log history save failures, clarify LAUNCHER_APP vs event namespace

LaunchHistory::save silently swallowed write errors; now shared by two
hosts (breadbox + breadbar's capsule), a broken cache dir silently
stopped recording launches for both. Log on failure instead.

do_launch/emit_launched's app_id parameter is the caller's bread
event-namespace id (breadbox passes "box"), not LAUNCHER_APP
("breadbox", scoped to cache/history paths only) - two similarly
named but distinct identities. Make the doc comments say so explicitly
and add tests pinning the exact namespace-check relationship, since
confusing them silently drops the emitted event.

bread-launcher/src/history.rs

@@ -25,16 +25,34 @@ impl LaunchHistory {
         *self.counts.entry(name.to_string()).or_insert(0) += 1;
     }
 
+    /// Writes `counts` to `path` as JSON. Best-effort — a broken cache dir
+    /// (missing parent, full disk, permissions) must not stop the caller
+    /// from launching anything, so this never returns an error — but it now
+    /// logs one on failure rather than swallowing it silently. Shared by two
+    /// hosts (breadbox's overlay and breadbar's embedded capsule, both keyed
+    /// under [`crate::LAUNCHER_APP`]), so a save failure here silently stops
+    /// ranking history for both.
     pub fn save(&self) {
-        if let Ok(json) = serde_json::to_string(&self.counts) {
-            let _ = fs::write(&self.path, json);
+        match serde_json::to_string(&self.counts) {
+            Ok(json) => {
+                if let Err(err) = fs::write(&self.path, json) {
+                    eprintln!(
+                        "bread-launcher: failed to save launch history to {}: {err}",
+                        self.path.display()
+                    );
+                }
+            }
+            Err(err) => {
+                eprintln!("bread-launcher: failed to serialize launch history: {err}");
+            }
         }
     }
 
-    /// In-memory history with no backing file — [`save`](Self::save) is a
-    /// silent no-op (an empty `path`). Lets a test (or a future in-memory
-    /// host) control counts directly instead of writing through
-    /// `~/.cache/<app>/history.json`.
+    /// In-memory history with no backing file — [`save`](Self::save) fails
+    /// (an empty `path` is not writable) and now logs that failure to
+    /// stderr rather than swallowing it, same as any other broken-path
+    /// case. Lets a test (or a future in-memory host) control counts
+    /// directly instead of writing through `~/.cache/<app>/history.json`.
     #[cfg(test)]
     pub(crate) fn from_counts(counts: HashMap<String, u32>) -> Self {
         LaunchHistory {
@@ -43,3 +61,66 @@ impl LaunchHistory {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn temp_history_path(name: &str) -> PathBuf {
+        let dir = std::env::temp_dir().join(format!(
+            "bread-launcher-history-test-{}-{name}-{}",
+            std::process::id(),
+            std::time::SystemTime::now()
+                .duration_since(std::time::UNIX_EPOCH)
+                .unwrap_or_default()
+                .as_nanos()
+        ));
+        std::fs::create_dir_all(&dir).unwrap();
+        dir.join("history.json")
+    }
+
+    #[test]
+    fn save_then_load_round_trips_counts() {
+        let path = temp_history_path("roundtrip");
+        let mut history = LaunchHistory {
+            counts: HashMap::new(),
+            path: path.clone(),
+        };
+        history.increment("firefox.desktop");
+        history.increment("firefox.desktop");
+        history.increment("kitty.desktop");
+        history.save();
+
+        let text = std::fs::read_to_string(&path).expect("save should have written the file");
+        let counts: HashMap<String, u32> = serde_json::from_str(&text).unwrap();
+        assert_eq!(counts.get("firefox.desktop"), Some(&2));
+        assert_eq!(counts.get("kitty.desktop"), Some(&1));
+
+        // load() from the same path should see the same counts.
+        let reloaded = LaunchHistory {
+            counts: serde_json::from_str(&std::fs::read_to_string(&path).unwrap()).unwrap(),
+            path: path.clone(),
+        };
+        assert_eq!(reloaded.count("firefox.desktop"), 2);
+
+        let _ = std::fs::remove_dir_all(path.parent().unwrap());
+    }
+
+    /// `save()` on an unwritable path (e.g. the parent directory doesn't
+    /// exist, or `path` is empty) must not panic — it's best-effort, called
+    /// from a launcher's shutdown path where a hard failure would be worse
+    /// than a lost history entry. This exercises exactly the failure branch
+    /// the `eprintln!` above was added for; there is no return value to
+    /// assert on, so "did not panic" is the contract under test.
+    #[test]
+    fn save_to_a_broken_path_does_not_panic() {
+        let history = LaunchHistory::from_counts(HashMap::from([("x".to_string(), 1)]));
+        history.save();
+
+        let history = LaunchHistory {
+            counts: HashMap::new(),
+            path: PathBuf::from("/nonexistent-dir/definitely-not-there/history.json"),
+        };
+        history.save();
+    }
+}

bread-launcher/src/launch.rs

@@ -25,6 +25,19 @@ fn pick_terminal() -> String {
 
 /// Spawns `entry`'s command (through a terminal if `entry.terminal` is set)
 /// and, on a successful spawn, publishes `event` via [`emit_launched`].
+///
+/// `app_id` here is the caller's **bread event-namespace id** (e.g.
+/// `"box"` for breadbox) — NOT [`crate::LAUNCHER_APP`] (`"breadbox"`).
+/// Those are two different identities that happen to look similar:
+/// `LAUNCHER_APP` only picks the shared cache/history directory (see its own
+/// doc comment), while `app_id` here is threaded straight into
+/// `BreadClient::connect(app_id)` and must be the caller's own namespace, or
+/// `BreadClient::emit`'s `validate_app_namespace` check
+/// (`event.starts_with("bread.{app_id}.")`) rejects `event` and drops it
+/// with only an eprintln — passing `LAUNCHER_APP` here by mistake is exactly
+/// that bug. breadbox passes its own `"box"` (see breadbox's `APP_ID`) so
+/// its events publish as `bread.box.*`, matching [`emit_launched`]'s doc
+/// example below.
 pub fn do_launch(entry: &DesktopEntry, app_id: &str, event: &str) {
     let cmd = entry.exec.trim();
     let spawned = if entry.terminal {
@@ -48,10 +61,14 @@ pub fn do_launch(entry: &DesktopEntry, app_id: &str, event: &str) {
     }
 }
 
-/// Publishes `event` (e.g. `bread.box.launched`) as `app_id` after a
-/// successful spawn. Fire-and-forget and non-fatal (`BreadClient::emit`
-/// never blocks or errors this caller) — breadd being absent must never
-/// affect launching itself.
+/// Publishes `event` under `app_id`'s bread namespace after a successful
+/// spawn — e.g. breadbox calls this with `app_id = "box"` and
+/// `event = "bread.box.launched"`, its own namespace. Fire-and-forget and
+/// non-fatal (`BreadClient::emit` never blocks or errors this caller) —
+/// breadd being absent must never affect launching itself. `app_id` must be
+/// the caller's *own* namespace id, not [`crate::LAUNCHER_APP`] — see
+/// [`do_launch`]'s doc comment for why those are different identities and
+/// what happens if they're confused.
 pub fn emit_launched(entry: &DesktopEntry, app_id: &str, event: &str) {
     let id = if entry.id.is_empty() {
         entry.exec.as_str()
@@ -63,3 +80,62 @@ pub fn emit_launched(entry: &DesktopEntry, app_id: &str, event: &str) {
         serde_json::json!({ "id": id, "name": entry.name }),
     );
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// Mirrors `bread_shared::apps::validate_app_namespace` exactly
+    /// (`event.starts_with(&format!("bread.{app}."))`) without pulling in
+    /// that crate here — this is the one check that decides whether
+    /// [`emit_launched`]'s event actually gets published.
+    fn passes_namespace_check(app_id: &str, event: &str) -> bool {
+        event.starts_with(&format!("bread.{app_id}."))
+    }
+
+    #[test]
+    fn documented_app_id_and_event_pair_passes_the_namespace_check() {
+        // breadbox's real call site (breadbox/breadbox/src/main.rs):
+        // APP_ID = "box", LAUNCHED_EVENT = "bread.box.launched".
+        assert!(
+            passes_namespace_check("box", "bread.box.launched"),
+            "do_launch/emit_launched's own doc example must actually pass \
+             BreadClient::emit's namespace check"
+        );
+    }
+
+    #[test]
+    fn launcher_app_is_not_a_valid_app_id_for_the_documented_event() {
+        // The historical bug this doc fix guards against: passing
+        // `LAUNCHER_APP` ("breadbox", the cache/history identity) as
+        // `app_id` instead of the caller's own namespace id ("box") would
+        // silently drop `bread.box.launched` — event.starts_with(
+        // "bread.breadbox.") is false for "bread.box.launched".
+        assert!(
+            !passes_namespace_check(crate::LAUNCHER_APP, "bread.box.launched"),
+            "LAUNCHER_APP must NOT satisfy the namespace check for the \
+             documented event — if this ever passes, do_launch's doc comment \
+             warning about confusing the two identities is wrong"
+        );
+    }
+
+    #[test]
+    fn emit_launched_does_not_panic_when_breadd_is_unreachable() {
+        // No daemon is running in a test environment — emit_launched (and
+        // the BreadClient::emit it wraps) must degrade silently rather than
+        // panicking or blocking, for both a valid and a namespace-violating
+        // app_id.
+        let entry = DesktopEntry {
+            id: "firefox.desktop".to_string(),
+            name: "Firefox".to_string(),
+            exec: "firefox".to_string(),
+            icon_name: String::new(),
+            icon_path: None,
+            categories: vec![],
+            wm_class: None,
+            terminal: false,
+        };
+        emit_launched(&entry, "box", "bread.box.launched");
+        emit_launched(&entry, crate::LAUNCHER_APP, "bread.box.launched");
+    }
+}

bread-launcher/src/lib.rs

@@ -53,4 +53,11 @@ pub use query::{builtin_commands, eval_calc, filter_commands, parse_query, Comma
 /// this constant — that reads exactly like an unfixed bug (breadbar naming
 /// another app's identity) and invites a later "fix" that would quietly
 /// break the shared history this constant exists to guarantee.
+///
+/// **Not** the `app_id` for [`do_launch`]/[`emit_launched`]: those publish
+/// bread-bus events, which must be namespaced under the caller's *own*
+/// identity (breadbox's is `"box"`, not `"breadbox"`) or
+/// `BreadClient::emit`'s namespace check silently drops them. This constant
+/// is scoped to the cache/history path family only — see [`do_launch`]'s
+/// doc comment for the concrete failure mode if the two get swapped.
 pub const LAUNCHER_APP: &str = "breadbox";