feat: Wire memory hygiene retention policy into heartbeat loop (#629)

* feat: Wire memory hygiene retention policy into heartbeat loop * review fix * linter fix * fix tests
2026-08-25 14:53:34 +00:00 · 2026-03-06 12:47:21 -08:00
parent 5869a9cc62
commit d195222124
5 changed files with 362 additions and 20 deletions
@@ -164,6 +164,7 @@ impl HeartbeatRunner {
                if report.had_work() {
                    tracing::info!(
                        daily_logs_deleted = report.daily_logs_deleted,
+                        conversation_docs_deleted = report.conversation_docs_deleted,
                        "heartbeat: memory hygiene deleted stale documents"
                    );
                }
@@ -10,8 +10,10 @@ use crate::error::ConfigError;
 pub struct HygieneConfig {
    /// Whether hygiene is enabled. Env: `MEMORY_HYGIENE_ENABLED` (default: true).
    pub enabled: bool,
-    /// Days before `daily/` documents are deleted. Env: `MEMORY_HYGIENE_RETENTION_DAYS` (default: 30).
-    pub retention_days: u32,
+    /// Days before `daily/` documents are deleted. Env: `MEMORY_HYGIENE_DAILY_RETENTION_DAYS` (default: 30).
+    pub daily_retention_days: u32,
+    /// Days before `conversations/` documents are deleted. Env: `MEMORY_HYGIENE_CONVERSATION_RETENTION_DAYS` (default: 7).
+    pub conversation_retention_days: u32,
    /// Minimum hours between hygiene passes. Env: `MEMORY_HYGIENE_CADENCE_HOURS` (default: 12).
    pub cadence_hours: u32,
 }
@@ -20,7 +22,8 @@ impl Default for HygieneConfig {
    fn default() -> Self {
        Self {
            enabled: true,
-            retention_days: 30,
+            daily_retention_days: 30,
+            conversation_retention_days: 7,
            cadence_hours: 12,
        }
    }
@@ -30,7 +33,11 @@ impl HygieneConfig {
    pub(crate) fn resolve() -> Result<Self, ConfigError> {
        Ok(Self {
            enabled: parse_bool_env("MEMORY_HYGIENE_ENABLED", true)?,
-            retention_days: parse_optional_env("MEMORY_HYGIENE_RETENTION_DAYS", 30)?,
+            daily_retention_days: parse_optional_env("MEMORY_HYGIENE_DAILY_RETENTION_DAYS", 30)?,
+            conversation_retention_days: parse_optional_env(
+                "MEMORY_HYGIENE_CONVERSATION_RETENTION_DAYS",
+                7,
+            )?,
            cadence_hours: parse_optional_env("MEMORY_HYGIENE_CADENCE_HOURS", 12)?,
        })
    }
@@ -40,7 +47,8 @@ impl HygieneConfig {
    pub fn to_workspace_config(&self) -> crate::workspace::hygiene::HygieneConfig {
        crate::workspace::hygiene::HygieneConfig {
            enabled: self.enabled,
-            retention_days: self.retention_days,
+            daily_retention_days: self.daily_retention_days,
+            conversation_retention_days: self.conversation_retention_days,
            cadence_hours: self.cadence_hours,
            state_dir: ironclaw_base_dir(),
        }
@@ -1,8 +1,8 @@
 //! Memory hygiene: automatic cleanup of stale workspace documents.
 //!
-//! Runs on a configurable cadence and deletes daily log entries older
-//! than the retention period. Identity files (`IDENTITY.md`, `SOUL.md`,
-//! etc.) are never touched.
+//! Runs on a configurable cadence and deletes daily log entries and conversation
+//! documents older than their respective retention periods. Identity files
+//! (`IDENTITY.md`, `SOUL.md`, etc.) are never touched.
 //!
 //! A global [`AtomicBool`] guard prevents concurrent hygiene passes, which
 //! avoids TOCTOU races on the state file and Windows file-locking errors
@@ -17,8 +17,10 @@
 //! │  1. Check cadence (skip if ran recently)     │
 //! │  2. Save state (claim the cadence window)    │
 //! │  3. List daily/ documents                    │
-//! │  4. Delete those older than retention_days   │
-//! │  5. Log summary                              │
+//! │  4. Delete those older than daily_retention  │
+//! │  5. List conversations/ documents            │
+//! │  6. Delete those older than conversation_ret │
+//! │  7. Log summary                              │
 //! └─────────────────────────────────────────────┘
 //! ```

@@ -34,13 +36,41 @@ use crate::workspace::Workspace;
 /// Global guard preventing concurrent hygiene passes.
 static RUNNING: AtomicBool = AtomicBool::new(false);

+/// Paths that must never be deleted by hygiene, regardless of age.
+const IDENTITY_PATHS: &[&str] = &[
+    crate::workspace::document::paths::MEMORY,
+    crate::workspace::document::paths::IDENTITY,
+    crate::workspace::document::paths::SOUL,
+    crate::workspace::document::paths::AGENTS,
+    crate::workspace::document::paths::USER,
+    crate::workspace::document::paths::HEARTBEAT,
+    crate::workspace::document::paths::README,
+    crate::workspace::document::paths::TOOLS,
+    crate::workspace::document::paths::BOOTSTRAP,
+];
+
+/// Check if a document path is an identity document that must never be deleted.
+///
+/// Performs case-insensitive comparison to handle case-insensitive filesystems
+/// (Windows, macOS) and prevent accidental deletion of identity docs with
+/// different casing (e.g., memory.md, MEMORY.MD, Memory.md).
+fn is_identity_path(path: &str) -> bool {
+    let file_name = path.rsplit('/').next().unwrap_or(path);
+    let file_name_lower = file_name.to_lowercase();
+    IDENTITY_PATHS
+        .iter()
+        .any(|&p| p.to_lowercase() == file_name_lower)
+}
+
 /// Configuration for workspace hygiene.
 #[derive(Debug, Clone)]
 pub struct HygieneConfig {
    /// Whether hygiene is enabled at all.
    pub enabled: bool,
    /// Documents in `daily/` older than this many days are deleted.
-    pub retention_days: u32,
+    pub daily_retention_days: u32,
+    /// Documents in `conversations/` older than this many days are deleted.
+    pub conversation_retention_days: u32,
    /// Minimum hours between hygiene passes.
    pub cadence_hours: u32,
    /// Directory to store state file (default: `~/.ironclaw`).
@@ -51,7 +81,8 @@ impl Default for HygieneConfig {
    fn default() -> Self {
        Self {
            enabled: true,
-            retention_days: 30,
+            daily_retention_days: 30,
+            conversation_retention_days: 7,
            cadence_hours: 12,
            state_dir: ironclaw_base_dir(),
        }
@@ -69,6 +100,8 @@ struct HygieneState {
 pub struct HygieneReport {
    /// Number of daily log documents deleted.
    pub daily_logs_deleted: u32,
+    /// Number of conversation documents deleted.
+    pub conversation_docs_deleted: u32,
    /// Whether the run was skipped (cadence not yet elapsed).
    pub skipped: bool,
 }
@@ -76,7 +109,7 @@ pub struct HygieneReport {
 impl HygieneReport {
    /// True if any cleanup work was done.
    pub fn had_work(&self) -> bool {
-        self.daily_logs_deleted > 0
+        self.daily_logs_deleted > 0 || self.conversation_docs_deleted > 0
    }
 }

@@ -136,21 +169,29 @@ pub async fn run_if_due(workspace: &Workspace, config: &HygieneConfig) -> Hygien
    save_state(&state_file);

    tracing::info!(
-        retention_days = config.retention_days,
+        daily_retention_days = config.daily_retention_days,
+        conversation_retention_days = config.conversation_retention_days,
        "memory hygiene: starting cleanup pass"
    );

    let mut report = HygieneReport::default();

    // Delete old daily logs
-    match cleanup_daily_logs(workspace, config.retention_days).await {
+    match cleanup_daily_logs(workspace, config.daily_retention_days).await {
        Ok(count) => report.daily_logs_deleted = count,
        Err(e) => tracing::warn!("memory hygiene: failed to clean daily logs: {e}"),
    }

+    // Delete old conversation documents
+    match cleanup_conversation_docs(workspace, config.conversation_retention_days).await {
+        Ok(count) => report.conversation_docs_deleted = count,
+        Err(e) => tracing::warn!("memory hygiene: failed to clean conversation docs: {e}"),
+    }
+
    if report.had_work() {
        tracing::info!(
            daily_logs_deleted = report.daily_logs_deleted,
+            conversation_docs_deleted = report.conversation_docs_deleted,
            "memory hygiene: cleanup complete"
        );
    } else {
@@ -183,6 +224,11 @@ async fn cleanup_daily_logs(
            continue;
        }

+        // Never delete identity documents
+        if is_identity_path(&entry.path) {
+            continue;
+        }
+
        // Check if the document is old enough to delete
        if let Some(updated_at) = entry.updated_at
            && updated_at < cutoff
@@ -205,6 +251,50 @@ async fn cleanup_daily_logs(
    Ok(deleted)
 }

+/// Delete conversation documents older than `retention_days`.
+async fn cleanup_conversation_docs(
+    workspace: &Workspace,
+    retention_days: u32,
+) -> Result<u32, anyhow::Error> {
+    let cutoff = Utc::now() - chrono::Duration::days(i64::from(retention_days));
+    let entries = workspace.list("conversations/").await?;
+
+    let mut deleted = 0u32;
+    for entry in entries {
+        if entry.is_directory {
+            continue;
+        }
+
+        // Never delete identity documents
+        if is_identity_path(&entry.path) {
+            continue;
+        }
+
+        // Check if the document is old enough to delete
+        if let Some(updated_at) = entry.updated_at
+            && updated_at < cutoff
+        {
+            let path = if entry.path.starts_with("conversations/") {
+                entry.path.clone()
+            } else {
+                format!("conversations/{}", entry.path)
+            };
+
+            if let Err(e) = workspace.delete(&path).await {
+                tracing::warn!(
+                    path,
+                    "memory hygiene: failed to delete conversation doc: {e}"
+                );
+            } else {
+                tracing::debug!(path, "memory hygiene: deleted old conversation doc");
+                deleted += 1;
+            }
+        }
+    }
+
+    Ok(deleted)
+}
+
 fn state_path_dir(state_file: &std::path::Path) -> Option<&std::path::Path> {
    state_file.parent()
 }
@@ -259,7 +349,8 @@ mod tests {
    fn default_config_is_reasonable() {
        let cfg = HygieneConfig::default();
        assert!(cfg.enabled);
-        assert_eq!(cfg.retention_days, 30);
+        assert_eq!(cfg.daily_retention_days, 30);
+        assert_eq!(cfg.conversation_retention_days, 7);
        assert_eq!(cfg.cadence_hours, 12);
    }

@@ -274,11 +365,83 @@ mod tests {
    fn report_had_work_when_deleted() {
        let report = HygieneReport {
            daily_logs_deleted: 3,
+            conversation_docs_deleted: 0,
            skipped: false,
        };
        assert!(report.had_work());
    }

+    #[test]
+    fn report_had_work_when_conversation_deleted() {
+        let report = HygieneReport {
+            daily_logs_deleted: 0,
+            conversation_docs_deleted: 2,
+            skipped: false,
+        };
+        assert!(report.had_work());
+    }
+
+    #[test]
+    fn is_identity_path_excludes_sacred_docs() {
+        for name in [
+            "MEMORY.md",
+            "IDENTITY.md",
+            "SOUL.md",
+            "AGENTS.md",
+            "USER.md",
+            "HEARTBEAT.md",
+            "README.md",
+            "TOOLS.md",
+            "BOOTSTRAP.md",
+        ] {
+            assert!(is_identity_path(name), "{name} should be excluded");
+            assert!(
+                is_identity_path(&format!("conversations/{name}")),
+                "conversations/{name} should be excluded via path"
+            );
+        }
+    }
+
+    #[test]
+    fn is_identity_path_case_insensitive() {
+        // Verify case-insensitive matching for case-insensitive filesystems
+        assert!(
+            is_identity_path("memory.md"),
+            "lowercase memory.md should be excluded"
+        );
+        assert!(
+            is_identity_path("Memory.md"),
+            "mixed case Memory.md should be excluded"
+        );
+        assert!(
+            is_identity_path("MEMORY.MD"),
+            "uppercase MEMORY.MD should be excluded"
+        );
+        assert!(
+            is_identity_path("identity.md"),
+            "lowercase identity.md should be excluded"
+        );
+        assert!(
+            is_identity_path("conversations/soul.md"),
+            "conversations/soul.md should be excluded"
+        );
+        assert!(
+            is_identity_path("conversations/SOUL.MD"),
+            "conversations/SOUL.MD should be excluded"
+        );
+    }
+
+    #[test]
+    fn is_identity_path_allows_normal_docs() {
+        for path in [
+            "daily/2024-01-01.md",
+            "conversations/chat-abc.md",
+            "notes.md",
+        ] {
+            assert!(!is_identity_path(path), "{path} should not be excluded");
+        }
+    }
+
    #[test]
    fn load_state_returns_none_for_missing_file() {
        assert!(load_state(std::path::Path::new("/tmp/nonexistent_hygiene.json")).is_none());
@@ -328,6 +491,9 @@ mod tests {
    fn running_guard_prevents_reentry() {
        let _lock = RUNNING_TESTS.lock().unwrap();

+        // Reset the global flag to ensure a clean state
+        RUNNING.store(false, Ordering::SeqCst);
+
        // Simulate acquiring the guard
        assert!(
            RUNNING
@@ -356,4 +522,168 @@ mod tests {
        );
        RUNNING.store(false, Ordering::SeqCst);
    }
+
+    // ================================================================
+    // Async integration tests (require libsql backend)
+    // ================================================================
+
+    #[cfg(feature = "libsql")]
+    mod async_tests {
+        use super::*;
+        use crate::db::Database;
+        use std::sync::Arc;
+
+        /// Helper to create a test database with migrations.
+        async fn create_test_db() -> (Arc<dyn crate::db::Database>, tempfile::TempDir) {
+            use crate::db::libsql::LibSqlBackend;
+
+            let temp_dir = tempfile::tempdir().expect("tempdir");
+            let db_path = temp_dir.path().join("test_hygiene.db");
+            let backend = LibSqlBackend::new_local(&db_path)
+                .await
+                .expect("LibSqlBackend::new_local");
+            backend.run_migrations().await.expect("run_migrations");
+            let db: Arc<dyn Database> = Arc::new(backend);
+            (db, temp_dir)
+        }
+
+        /// Helper to create a workspace from a test database.
+        fn create_workspace(db: &Arc<dyn Database>) -> Arc<Workspace> {
+            Arc::new(Workspace::new_with_db("default", db.clone()))
+        }
+
+        #[tokio::test]
+        async fn cleanup_daily_logs_preserves_identity_documents() {
+            let (db, _tmp) = create_test_db().await;
+            let ws = create_workspace(&db);
+
+            // Write several regular documents (non-identity)
+            ws.write("daily/2024-01-15.md", "Old log")
+                .await
+                .expect("write log 1");
+            ws.write("daily/2024-01-20.md", "Another log")
+                .await
+                .expect("write log 2");
+
+            // Write an identity document
+            ws.write("MEMORY.md", "Long-term curated memory")
+                .await
+                .expect("write identity");
+
+            // List before cleanup
+            let before = ws.list("daily/").await.expect("list before");
+            let daily_count_before = before.iter().filter(|e| !e.is_directory).count();
+            assert!(daily_count_before >= 2, "should have at least 2 daily logs");
+
+            // Run cleanup with 0-day retention (deletes everything old)
+            // This tests that even with aggressive cleanup, identity docs survive
+            let deleted = cleanup_daily_logs(&ws, 0)
+                .await
+                .expect("cleanup_daily_logs");
+
+            // Should have deleted some documents (the daily logs)
+            assert!(deleted > 0, "should have deleted old daily documents");
+
+            // Verify identity doc still exists
+            let identity = db
+                .get_document_by_path("default", None, "MEMORY.md")
+                .await
+                .expect("get identity doc");
+            assert_eq!(identity.path, "MEMORY.md");
+            assert_eq!(identity.content, "Long-term curated memory");
+        }
+
+        #[tokio::test]
+        async fn cleanup_conversation_docs_handles_empty_directory() {
+            let (db, _tmp) = create_test_db().await;
+            let ws = create_workspace(&db);
+
+            // Run cleanup on an empty directory (conversations/ doesn't exist)
+            let deleted = cleanup_conversation_docs(&ws, 7)
+                .await
+                .expect("cleanup_conversation_docs");
+
+            // Should delete 0 (nothing to delete)
+            assert_eq!(deleted, 0, "should delete 0 from empty directory");
+        }
+
+        #[tokio::test]
+        async fn cleanup_respects_cadence_prevents_concurrent_runs() {
+            let (db, _tmp) = create_test_db().await;
+            let ws = create_workspace(&db);
+
+            let config = HygieneConfig {
+                enabled: true,
+                daily_retention_days: 30,
+                conversation_retention_days: 7,
+                cadence_hours: 12,
+                state_dir: _tmp.path().to_path_buf(),
+            };
+
+            // First run should succeed
+            let report1 = run_if_due(&ws, &config).await;
+            assert!(!report1.skipped, "first run should not be skipped");
+
+            // Second run immediately should be skipped (cadence not elapsed)
+            let report2 = run_if_due(&ws, &config).await;
+            assert!(report2.skipped, "second run should be skipped by cadence");
+
+            // Report structure should be correct
+            assert_eq!(
+                report1.daily_logs_deleted + report1.conversation_docs_deleted,
+                0,
+                "first run should have clean counts"
+            );
+        }
+
+        #[tokio::test]
+        async fn cleanup_reports_deletion_counts_correctly() {
+            let (db, _tmp) = create_test_db().await;
+            let ws = create_workspace(&db);
+
+            // Write some documents
+            ws.write("daily/log1.md", "content 1")
+                .await
+                .expect("write doc 1");
+            ws.write("daily/log2.md", "content 2")
+                .await
+                .expect("write doc 2");
+            ws.write("conversations/chat1.md", "content 3")
+                .await
+                .expect("write doc 3");
+
+            // Run with 0-day retention to delete everything non-identity
+            let deleted_daily = cleanup_daily_logs(&ws, 0).await.expect("cleanup daily");
+            let deleted_conv = cleanup_conversation_docs(&ws, 0)
+                .await
+                .expect("cleanup conversations");
+
+            // Both should report deletions
+            assert!(deleted_daily > 0, "should report deleted daily logs");
+            assert_eq!(deleted_conv, 1, "should report 1 deleted conversation doc");
+
+            // Create a HygieneReport and verify aggregation works
+            let report = HygieneReport {
+                daily_logs_deleted: deleted_daily,
+                conversation_docs_deleted: deleted_conv,
+                skipped: false,
+            };
+
+            // Verify HygieneReport structure
+            assert!(!report.skipped, "should not be skipped");
+            assert!(report.had_work(), "report should indicate work was done");
+            assert!(
+                report.daily_logs_deleted > 0 || report.conversation_docs_deleted > 0,
+                "report should have at least one deletion count > 0"
+            );
+
+            // Verify had_work() correctly combines both counts
+            let no_work = HygieneReport {
+                daily_logs_deleted: 0,
+                conversation_docs_deleted: 0,
+                skipped: false,
+            };
+            assert!(!no_work.had_work(), "empty report should indicate no work");
+        }
+    }
 }