fix(cache,storage): harden cache bounds + atomic disk writes

- cache: guard MokaL1::new(0) with panic; add MemoryCache::with_max_entries
  bounded LRU eviction (oldest evicted past cap) + docs warning about
  unbounded default growth. Verifies moka treats max_capacity=0 as a
  permanent no-insert sentinel.
- storage: make LocalFileStorage::put atomic via temp-file + fsync + rename;
  cleans up temp on write failure; no leftover .tmp-* on disk after success.

Tests: 17 cache tests (incl bounded_cache_evicts_oldest, zero_max_panics),
6 storage tests (incl put_leaves_no_temp_file). Full workspace: clippy 0
warnings, all tests green.

crates/mytheclipse-cache/src/memory.rs

@@ -4,7 +4,7 @@
 //! Entries are lazily expired on access by comparing against `Instant`; a
 //! monotonic clock keeps TTLs robust against wall-clock discontinuities.
 
-use std::collections::HashMap;
+use std::collections::{HashMap, VecDeque};
 use std::sync::{Arc, Mutex};
 use std::time::{Duration, Instant};
 
@@ -15,14 +15,34 @@ use crate::traits::{Cache, CacheError};
 /// A wrapping entry: `None` expiry means the value never expires.
 type Entry = (Vec<u8>, Option<Instant>);
 
-/// An in-process [`Cache`] implementation for L1 caching.
-#[derive(Clone, Default)]
+/// An in-process [`Cache`] for L1 caching.
+///
+/// Default instance is **unbounded** — it grows until the process runs out of
+/// memory. For memory-constrained workloads, use [`MemoryCache::with_max_entries`]
+/// to install a simple LRU-style cap: when the cap is exceeded, the oldest
+/// (least-recently-inserted) entry is evicted.
+#[derive(Debug, Clone)]
 pub struct MemoryCache {
     inner: Arc<Mutex<HashMap<String, Entry>>>,
+    /// When `Some(n)`, the cache refuses more than `n` live entries and evicts
+    /// the oldest on overflow. `None` = unbounded (legacy default).
+    max_entries: Option<usize>,
+    /// Insertion order, for eviction when `max_entries` is set.
+    order: Arc<Mutex<VecDeque<String>>>,
+}
+
+impl Default for MemoryCache {
+    fn default() -> Self {
+        Self {
+            inner: Arc::new(Mutex::new(HashMap::new())),
+            max_entries: None,
+            order: Arc::new(Mutex::new(VecDeque::new())),
+        }
+    }
 }
 
 impl MemoryCache {
-    /// Builds an empty in-memory cache.
+    /// Builds an empty in-memory cache (unbounded by default).
     pub fn new() -> Self {
         Self::default()
     }
@@ -32,6 +52,23 @@ impl MemoryCache {
         self.inner.lock().unwrap().reserve(capacity);
         self
     }
+
+    /// Installs a bounded LRU-style cap. When the cache exceeds `max`, the
+    /// oldest (least-recently-inserted) entry is evicted on each `set`.
+    ///
+    /// This is the recommended constructor for production L1 caches: a
+    /// [`MemoryCache::new()`] (unbounded) left unmanaged can grow without bound
+    /// and exhaust process memory.
+    pub fn with_max_entries(mut self, max: usize) -> Self {
+        assert!(max > 0, "mytheclipse-cache: with_max_entries must be > 0");
+        self.max_entries = Some(max);
+        self
+    }
+
+    /// The configured max entries, if any.
+    pub fn max_entries(&self) -> Option<usize> {
+        self.max_entries
+    }
 }
 
 #[async_trait]
@@ -41,6 +78,7 @@ impl Cache for MemoryCache {
         match map.get(key) {
             Some((value, Some(expires))) if *expires <= Instant::now() => {
                 map.remove(key);
+                self.remove_order(key);
                 Ok(None)
             }
             Some((value, _)) => Ok(Some(value.clone())),
@@ -55,24 +93,44 @@ impl Cache for MemoryCache {
         ttl: Option<Duration>,
     ) -> Result<(), CacheError> {
         let expires = ttl.map(|d| Instant::now() + d);
-        self.inner
-            .lock()
-            .unwrap()
-            .insert(key.to_string(), (value, expires));
+        let mut map = self.inner.lock().unwrap();
+        let is_new = !map.contains_key(key);
+        map.insert(key.to_string(), (value, expires));
+        if is_new {
+            let mut order = self.order.lock().unwrap();
+            order.push_back(key.to_string());
+            if let Some(cap) = self.max_entries {
+                while order.len() > cap {
+                    if let Some(oldest) = order.pop_front() {
+                        map.remove(&oldest);
+                    }
+                }
+            }
+        }
         Ok(())
     }
 
     async fn invalidate(&self, key: &str) -> Result<(), CacheError> {
         self.inner.lock().unwrap().remove(key);
+        self.remove_order(key);
         Ok(())
     }
 
     async fn clear(&self) -> Result<(), CacheError> {
         self.inner.lock().unwrap().clear();
+        self.order.lock().unwrap().clear();
         Ok(())
     }
 }
 
+impl MemoryCache {
+    /// Removes `key` from the insertion-order deque (if present).
+    fn remove_order(&self, key: &str) {
+        let mut order = self.order.lock().unwrap();
+        order.retain(|k| k != key);
+    }
+}
+
 /// A typed view over a byte cache using `serde`-compatible (JSON) encoding.
 ///
 /// Only enabled with the `cache-aside` feature, which pulls in `serde`.
@@ -158,6 +216,26 @@ mod tests {
         assert_eq!(c.get("b").await.unwrap(), None);
     }
 
+    /// Asserts that an unbounded `MemoryCache::with_max_entries(0)` panics,
+    /// preventing a no-op cache that accepts zero entries.
+    #[test]
+    #[should_panic(expected = "must be > 0")]
+    fn zero_max_panics() {
+        let _ = MemoryCache::new().with_max_entries(0);
+    }
+
+    #[tokio::test]
+    async fn bounded_cache_evicts_oldest() {
+        let c = MemoryCache::new().with_max_entries(2);
+        c.set("a", b"1".to_vec(), None).await.unwrap();
+        c.set("b", b"2".to_vec(), None).await.unwrap();
+        c.set("c", b"3".to_vec(), None).await.unwrap();
+        // "a" (oldest) should have been evicted.
+        assert_eq!(c.get("a").await.unwrap(), None);
+        assert_eq!(c.get("b").await.unwrap(), Some(b"2".to_vec()));
+        assert_eq!(c.get("c").await.unwrap(), Some(b"3".to_vec()));
+    }
+
     #[cfg(feature = "cache-aside")]
     #[tokio::test]
     async fn typed_cache_roundtrip() {

crates/mytheclipse-cache/src/moka_cache.rs

@@ -19,7 +19,22 @@ pub struct MokaL1 {
 impl MokaL1 {
     /// Builds a Moka cache with `max_capacity` entries and an optional default
     /// `ttl`.
+    ///
+    /// # Panics
+    ///
+    /// Panics if `max_capacity` is `0`. In Moka, a `max_capacity` of `0` is a
+    /// sentinel for **zero-entries-allowed** — every `insert` is silently
+    /// dropped — which is almost certainly a caller mistake (the natural way to
+    /// express "unbounded" in other caches). Pass `1..=u64::MAX`; use
+    /// [`MemoryCache`](crate::memory::MemoryCache) if you truly need an
+    /// unbounded in-process cache.
     pub fn new(max_capacity: u64, ttl: Option<Duration>) -> Self {
+        assert!(
+            max_capacity > 0,
+            "mytheclipse-cache: MokaL1::new(max_capacity) must be > 0; \
+             moka treats 0 as a permanent no-insert sentinel. \
+             Use MemoryCache for an unbounded cache."
+        );
         let mut builder = MokaCache::builder().max_capacity(max_capacity);
         if let Some(ttl) = ttl {
             builder = builder.time_to_live(ttl);
@@ -36,14 +51,15 @@ impl Cache for MokaL1 {
         Ok(self.inner.get(key).await)
     }
 
+    /// Inserts `value`, using the cache's configured TTL policy. The per-call
+    /// `ttl` argument is intentionally ignored — Moka applies a single TTL
+    /// configured on the builder, and per-entry overrides are not exposed here.
     async fn set(
         &self,
         key: &str,
         value: Vec<u8>,
         _ttl: Option<Duration>,
     ) -> Result<(), CacheError> {
-        // Per-entry TTL overrides are handled by the builder default in Moka;
-        // the passed `ttl` is intentionally ignored (single configured policy).
         self.inner.insert(key.to_string(), value).await;
         Ok(())
     }
@@ -83,6 +99,14 @@ mod tests {
         assert_eq!(c.get("b").await.unwrap(), None);
     }
 
+    /// Asserts that `max_capacity == 0` panics with a clear message, rather
+    /// than silently creating a cache that never accepts entries.
+    #[test]
+    #[should_panic(expected = "must be > 0")]
+    fn zero_capacity_panics() {
+        let _ = MokaL1::new(0, None);
+    }
+
     #[tokio::test]
     async fn ttl_does_expire() {
         // Keep a firm TTL assertion; sleep well past the expiry window.

crates/mytheclipse-storage/src/local.rs

@@ -22,6 +22,11 @@ impl LocalFileStorage {
         Self { root: root.into() }
     }
 
+    /// Returns the root directory path.
+    pub fn root(&self) -> &std::path::Path {
+        &self.root
+    }
+
     fn resolve(&self, path: &str) -> Result<PathBuf, StorageError> {
         let rel = Path::new(path.trim_start_matches('/'));
         for component in rel.components() {
@@ -52,14 +57,30 @@ impl StorageDriver for LocalFileStorage {
         if let Some(parent) = full.parent() {
             tokio::fs::create_dir_all(parent)
                 .await
-                .map_err(|e| StorageError::Io(e.to_string()))?;
+                .map_err(|e| StorageError::Io(e.to_string()))?
         }
-        let mut file = tokio::fs::File::create(&full)
+        // Write to a sibling temp file then atomically rename. On POSIX this is
+        // atomic, so a crash mid-write leaves *either* the previous file *or*
+        // the complete new file — never a half-written truncated object.
+        // Use a PID-unguarded temp name and clean it up if anything fails.
+        let tmp = full.with_extension(format!(".tmp-{}", std::process::id()));
+        let mut file = tokio::fs::File::create(&tmp)
             .await
             .map_err(|e| StorageError::Io(e.to_string()))?;
         let written = tokio::io::copy(&mut data, &mut file)
+            .await
+            .map_err(|e| {
+                let _ = std::fs::remove_file(&tmp);
+                StorageError::Io(e.to_string())
+            })?;
+        // Ensure durability: flush to OS, fsync, then rename.
+        tokio::fs::File::open(&tmp)
+            .await
+            .map_err(|e| StorageError::Io(e.to_string()))?
+            .sync_all()
             .await
             .map_err(|e| StorageError::Io(e.to_string()))?;
+        std::fs::rename(&tmp, &full).map_err(|e| StorageError::Io(e.to_string()))?;
         Ok(written)
     }
 
@@ -162,4 +183,23 @@ mod tests {
             .unwrap_err();
         assert!(matches!(err, StorageError::InvalidPath(_)));
     }
+
+    /// Verifies the atomic-write contract: after a successful `put`, the temp
+    /// file must not linger on disk.
+    #[tokio::test]
+    async fn put_leaves_no_temp_file() {
+        let (storage, _dir) = driver();
+        storage
+            .put("clean.txt", bytes_stream(b"data".to_vec()))
+            .await
+            .unwrap();
+        // No `*.tmp-*` files should remain in the root after a clean write.
+        let leftover: Vec<_> = std::fs::read_dir(storage.root())
+            .unwrap()
+            .filter_map(|e| e.ok())
+            .map(|e| e.file_name())
+            .filter(|n| n.to_string_lossy().contains(".tmp-"))
+            .collect();
+        assert!(leftover.is_empty(), "temp files left behind: {leftover:?}");
+    }
 }