diff --git a/.env.example b/.env.example index 9fe1f460..2022c8a7 100644 --- a/.env.example +++ b/.env.example @@ -108,8 +108,9 @@ HEARTBEAT_NOTIFY_USER=default # Memory hygiene settings (automatic cleanup of stale workspace documents) # Runs on each heartbeat tick; identity files (IDENTITY.md, SOUL.md) are never deleted # MEMORY_HYGIENE_ENABLED=true -# MEMORY_HYGIENE_RETENTION_DAYS=30 # delete daily/ docs older than this many days -# MEMORY_HYGIENE_CADENCE_HOURS=12 # minimum hours between cleanup passes +# MEMORY_HYGIENE_DAILY_RETENTION_DAYS=30 # delete daily/ docs older than this many days +# MEMORY_HYGIENE_CONVERSATION_RETENTION_DAYS=7 # delete conversations/ docs older than this many days +# MEMORY_HYGIENE_CADENCE_HOURS=12 # minimum hours between cleanup passes # Safety settings SAFETY_MAX_OUTPUT_LENGTH=100000 diff --git a/src/agent/heartbeat.rs b/src/agent/heartbeat.rs index be721b34..34f56a5c 100644 --- a/src/agent/heartbeat.rs +++ b/src/agent/heartbeat.rs @@ -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" ); } diff --git a/src/config/hygiene.rs b/src/config/hygiene.rs index 426bf7ec..b510933a 100644 --- a/src/config/hygiene.rs +++ b/src/config/hygiene.rs @@ -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 { 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(), } diff --git a/src/workspace/hygiene.rs b/src/workspace/hygiene.rs index 9e6fc852..d84d8f03 100644 --- a/src/workspace/hygiene.rs +++ b/src/workspace/hygiene.rs @@ -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 { + 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, 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 = Arc::new(backend); + (db, temp_dir) + } + + /// Helper to create a workspace from a test database. + fn create_workspace(db: &Arc) -> Arc { + 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"); + } + } } diff --git a/tests/e2e_routine_heartbeat.rs b/tests/e2e_routine_heartbeat.rs index 929f8715..6f4dda34 100644 --- a/tests/e2e_routine_heartbeat.rs +++ b/tests/e2e_routine_heartbeat.rs @@ -348,7 +348,8 @@ mod tests { let hygiene_config = HygieneConfig { enabled: false, - retention_days: 30, + daily_retention_days: 30, + conversation_retention_days: 7, cadence_hours: 24, state_dir: _tmp.path().to_path_buf(), }; @@ -399,7 +400,8 @@ mod tests { let hygiene_config = HygieneConfig { enabled: false, - retention_days: 30, + daily_retention_days: 30, + conversation_retention_days: 7, cadence_hours: 24, state_dir: _tmp.path().to_path_buf(), };