breadpad: save note before window can close, add network timeouts, lock the JSONL store, fix monthly recurrence, restrict config permissions

- main.rs: hold the gtk4::Application (RAII guard from app.hold()) across
  the idle_add_local_once save callback, so closing the capture window
  can no longer let the process quit before the note is actually written
  to disk
- breadpad-shared/src/ai.rs: 10s timeout on the Ollama HTTP call (ureq had
  none, so a hung local endpoint could stall indefinitely)
- breadpad-shared/src/calendar.rs: 15s timeout on the CalDAV reqwest
  client (applies to every CalDAV request made through it)
- breadpad-shared/src/store.rs: exclusive flock (std::fs::File::lock, no
  new dependency needed at this Rust version) on a sidecar lock file
  around every read-modify-write span, guarding against breadpad/breadman/
  reminder-fire processes racing each other on notes.jsonl
- breadpad-shared/src/scheduler.rs: parse_next_from_rrule now handles
  FREQ=MONTHLY (previously fell into the catch-all None arm, so a monthly
  reminder fired once and never rescheduled); reachable today via
  breadman's free-text RRULE editor field
- breadpad-shared/src/config.rs: Config::save() chmods breadpad.toml to
  0600 after writing, since it can hold the CalDAV password in plaintext

Pre-existing, unrelated test failure noted: theme::tests::css_defines_bg_color
fails identically on the original commit (depends on this machine's live
pywal cache) — confirmed via git stash, not touched.

breadpad-shared/src/ai.rs

@@ -61,8 +61,14 @@ impl OllamaClient {
             "stream": false
         });
 
+        // ureq 2's default agent has no overall request timeout, so a hung
+        // local Ollama endpoint would otherwise stall this call forever —
+        // and since classification now runs from an idle callback after the
+        // capture window has already closed (see `main.rs`), a hang here is
+        // invisible to the user, not just slow.
         let response = ureq::post(&url)
             .set("Content-Type", "application/json")
+            .timeout(std::time::Duration::from_secs(10))
             .send_json(payload)
             .map_err(|e| anyhow::anyhow!("Ollama HTTP error: {}", e))?;
 

breadpad-shared/src/calendar.rs

@@ -18,7 +18,13 @@ impl CalDavClient {
     pub fn new(config: CalendarConfig) -> Self {
         // `reqwest::Client::builder().build()` can only fail if the TLS backend can't be
         // initialised; fall back to `Client::new()` semantics rather than panicking.
+        //
+        // A request-wide timeout is set here (rather than on `Client::new()`'s
+        // untimed defaults) so a hung/unreachable CalDAV server can't hang
+        // whatever's making the request indefinitely — `reqwest::Client::new()`
+        // has no timeout of its own.
         let client = reqwest::Client::builder()
+            .timeout(std::time::Duration::from_secs(15))
             .build()
             .unwrap_or_else(|e| {
                 tracing::warn!("falling back to default HTTP client: {}", e);

breadpad-shared/src/config.rs

@@ -193,6 +193,20 @@ impl Config {
         }
         let text = toml::to_string_pretty(self)?;
         fs::write(&path, text)?;
+
+        // This file can hold the CalDAV password in plaintext (see
+        // `CalendarConfig`'s own doc comment) — `fs::write` creates it with
+        // the process's default umask, which on most setups means
+        // world-readable. Lock it down to owner-only rather than just
+        // telling the user to do it themselves.
+        #[cfg(unix)]
+        {
+            use std::os::unix::fs::PermissionsExt;
+            if let Err(e) = fs::set_permissions(&path, fs::Permissions::from_mode(0o600)) {
+                tracing::warn!("failed to restrict permissions on {}: {}", path.display(), e);
+            }
+        }
+
         Ok(())
     }
 }

breadpad-shared/src/scheduler.rs

@@ -206,6 +206,42 @@ pub(crate) fn parse_next_from_rrule(rrule_str: &str, default_morning: &str) -> O
                 (now.date_naive() + chrono::Duration::days(days_ahead)).and_time(fire_time);
             return Some(local_naive_to_utc(target_date));
         }
+        "MONTHLY" => {
+            use chrono::Datelike;
+            // BYMONTHDAY isn't guaranteed to be present — breadman's note
+            // editor lets a user type an arbitrary RRULE by hand, and
+            // "FREQ=MONTHLY" alone is a perfectly valid (if under-specified)
+            // one. Fall back to today's day-of-month, mirroring how WEEKLY
+            // above defaults BYDAY to "MO" when absent.
+            let day: u32 = parts
+                .get("BYMONTHDAY")
+                .and_then(|v| v.parse().ok())
+                .filter(|d: &u32| (1..=31).contains(d))
+                .unwrap_or_else(|| now.day());
+
+            let mut year = now.year();
+            let mut month = now.month();
+
+            // Walk forward month by month for the next calendar date that
+            // (a) actually has this day-of-month (a 31st skips e.g. April)
+            // and (b) is still in the future. Bounded to 24 months as a
+            // defensive cap — every valid day (1-31) recurs well within a
+            // year, so this should never come close to firing.
+            for _ in 0..24 {
+                if let Some(date) = chrono::NaiveDate::from_ymd_opt(year, month, day) {
+                    let candidate = date.and_time(fire_time);
+                    if now.naive_local() < candidate {
+                        return Some(local_naive_to_utc(candidate));
+                    }
+                }
+                month += 1;
+                if month > 12 {
+                    month = 1;
+                    year += 1;
+                }
+            }
+            None
+        }
         _ => None,
     }
 }
@@ -404,7 +440,40 @@ mod tests {
 
     #[test]
     fn unknown_freq_returns_none() {
-        assert!(parse_next_from_rrule("RRULE:FREQ=MONTHLY;BYHOUR=9;BYMINUTE=0", "08:00").is_none());
+        assert!(parse_next_from_rrule("RRULE:FREQ=YEARLY;BYHOUR=9;BYMINUTE=0", "08:00").is_none());
+    }
+
+    #[test]
+    fn monthly_reschedules_instead_of_returning_none() {
+        // This used to be the exact bug: MONTHLY fell into the `_ => None`
+        // arm, so a monthly reminder fired once and never rescheduled.
+        let t = parse_next_from_rrule("RRULE:FREQ=MONTHLY;BYMONTHDAY=15;BYHOUR=9;BYMINUTE=0", "08:00");
+        assert!(t.is_some(), "MONTHLY must produce a next occurrence, not None");
+        let local: chrono::DateTime<Local> = t.unwrap().into();
+        assert_eq!(local.day(), 15);
+        assert_eq!(local.hour(), 9);
+        assert_eq!(local.minute(), 0);
+        assert!(local > Local::now());
+    }
+
+    #[test]
+    fn monthly_without_bymonthday_uses_todays_day_of_month() {
+        let t = parse_next_from_rrule("RRULE:FREQ=MONTHLY;BYHOUR=23;BYMINUTE=59", "08:00").unwrap();
+        let local: chrono::DateTime<Local> = t.into();
+        assert_eq!(local.day(), Local::now().day());
+    }
+
+    #[test]
+    fn monthly_on_the_31st_skips_shorter_months() {
+        // Every candidate month/day combination this walks must actually
+        // exist (from_ymd_opt returning None for e.g. April 31 is skipped
+        // internally) — this mostly guards against a panic/infinite loop
+        // regression rather than a specific date, since "next Feb 31" et al
+        // must fall through to a month that really has a 31st.
+        let t = parse_next_from_rrule("RRULE:FREQ=MONTHLY;BYMONTHDAY=31;BYHOUR=9;BYMINUTE=0", "08:00");
+        assert!(t.is_some());
+        let local: chrono::DateTime<Local> = t.unwrap().into();
+        assert_eq!(local.day(), 31);
     }
 
     #[test]

breadpad-shared/src/store.rs

@@ -44,11 +44,43 @@ impl Store {
         }
     }
 
+    /// Path to the sidecar lock file guarding the whole note store — a
+    /// single lock covers both `notes.jsonl` and `archive.jsonl` since
+    /// `rotate_archive` moves notes between them in one logical operation.
+    fn lock_path(&self) -> PathBuf {
+        self.notes_path.with_extension("lock")
+    }
+
+    /// Blocks until an exclusive advisory lock (`flock`) is held on the
+    /// sidecar lock file, and holds it for as long as the returned `File`
+    /// stays alive (the lock is released automatically when it's dropped,
+    /// same as it would be on process exit/crash).
+    ///
+    /// breadpad (capture/fire/snooze), breadman (edit), and multiple
+    /// concurrent reminder-fire processes all read and rewrite the same
+    /// JSONL file with no coordination otherwise: two concurrent
+    /// load-modify-rewrite cycles racing `write_all`'s rename can silently
+    /// lose one side's change. This is intentionally one lock for the
+    /// entire store rather than per-note or per-file locking — contention
+    /// is expected to be rare (a handful of short-lived CLI-ish processes,
+    /// not a server), so simplicity wins over fine-grained locking here.
+    fn acquire_lock(&self) -> Result<fs::File> {
+        let file = OpenOptions::new()
+            .create(true)
+            .write(true)
+            .open(self.lock_path())
+            .context("failed to open notes store lock file")?;
+        file.lock().context("failed to acquire notes store lock")?;
+        Ok(file)
+    }
+
     pub fn load_all(&self) -> Result<Vec<Note>> {
+        let _lock = self.acquire_lock()?;
         self.load_from(&self.notes_path)
     }
 
     pub fn load_archive(&self) -> Result<Vec<Note>> {
+        let _lock = self.acquire_lock()?;
         self.load_from(&self.archive_path)
     }
 
@@ -74,12 +106,15 @@ impl Store {
     }
 
     pub fn save_note(&self, note: &Note) -> Result<()> {
+        {
+            let _lock = self.acquire_lock()?;
             let mut file = OpenOptions::new()
                 .create(true)
                 .append(true)
                 .open(&self.notes_path)?;
             let line = serde_json::to_string(note)?;
             writeln!(file, "{}", line)?;
+        }
 
         if let Some(cal_cfg) = &self.calendar {
             if cal_cfg.enabled && (note.time.is_some() || note.rrule.is_some()) {
@@ -103,13 +138,18 @@ impl Store {
     }
 
     pub fn delete_note(&self, id: &str) -> Result<()> {
-        let all = self.load_all()?;
-        let (to_delete, keep): (Vec<Note>, Vec<Note>) = all.into_iter().partition(|n| n.id == id);
+        let to_delete_note = {
+            let _lock = self.acquire_lock()?;
+            let all = self.load_from(&self.notes_path)?;
+            let (to_delete, keep): (Vec<Note>, Vec<Note>) =
+                all.into_iter().partition(|n| n.id == id);
             self.write_all(&self.notes_path, &keep)?;
+            to_delete.into_iter().next()
+        };
 
         if let Some(cal_cfg) = &self.calendar {
             if cal_cfg.enabled {
-                if let Some(note) = to_delete.into_iter().next() {
+                if let Some(note) = to_delete_note {
                     spawn_caldav_delete(caldav_uid(&note), cal_cfg.clone());
                 }
             }
@@ -118,11 +158,17 @@ impl Store {
         Ok(())
     }
 
+    /// Holds the store lock across the whole load-modify-write span (not
+    /// just the write) — `load_from` is used directly here rather than the
+    /// public, self-locking `load_all`, since re-acquiring the same
+    /// process-wide advisory lock while already holding it would block
+    /// forever (`flock` isn't re-entrant across separate file descriptors).
     fn rewrite_notes<F>(&self, mut f: F) -> Result<()>
     where
         F: FnMut(Note) -> Note,
     {
-        let notes: Vec<Note> = self.load_all()?.into_iter().map(|n| f(n)).collect();
+        let _lock = self.acquire_lock()?;
+        let notes: Vec<Note> = self.load_from(&self.notes_path)?.into_iter().map(|n| f(n)).collect();
         self.write_all(&self.notes_path, &notes)
     }
 
@@ -141,8 +187,9 @@ impl Store {
     }
 
     pub fn rotate_archive(&self, archive_after_days: i64) -> Result<usize> {
+        let _lock = self.acquire_lock()?;
         let cutoff = Utc::now() - Duration::days(archive_after_days);
-        let notes = self.load_all()?;
+        let notes = self.load_from(&self.notes_path)?;
         let (to_archive, keep): (Vec<Note>, Vec<Note>) = notes
             .into_iter()
             .partition(|n| n.done && n.completed.map_or(false, |c| c < cutoff));

breadpad/src/main.rs

@@ -694,6 +694,7 @@ fn build_window(
     let selected_type_clone = selected_type.clone();
     let cfg_clone = cfg.clone();
     let workspace_clone = workspace.clone();
+    let app_clone = app.clone();
 
     let save_and_close = {
         let win = win_clone.clone();
@@ -701,6 +702,7 @@ fn build_window(
         let selected_type = selected_type_clone.clone();
         let cfg = cfg_clone.clone();
         let workspace = workspace_clone.clone();
+        let app = app_clone.clone();
 
         move || {
             let text = entry.text().to_string();
@@ -711,10 +713,20 @@ fn build_window(
             let note_type = selected_type.borrow().clone();
             let cfg_c = cfg.clone();
             let ws_c = workspace.clone();
-            // Close first so the popup disappears immediately, then save.
+            // Close first so the popup disappears immediately, then save —
+            // but `hold()` the application across the gap first. Without
+            // this, closing the only open window can let the whole process
+            // (and with it, the `idle_add_local_once` callback below that
+            // actually writes the note) exit before that callback ever
+            // runs, silently losing the note the user just typed. `hold()`
+            // returns an RAII guard that keeps the app alive with zero
+            // windows open until it's dropped, right after the save
+            // finishes below.
+            let hold_guard = app.hold();
             win.close();
             glib::idle_add_local_once(move || {
                 save_note_classified(&text, note_type, no_classify, cfg_c, ws_c);
+                drop(hold_guard);
             });
         }
     };