diff --git a/config/example.toml b/config/example.toml index a78eb62..830507f 100644 --- a/config/example.toml +++ b/config/example.toml @@ -102,6 +102,6 @@ trigger_labels = ["coven:fix", "coven:review"] # Labels that trigger this famil # [memory] # enabled = false # approval_required = true # learned facts stay pending until a maintainer approves -# retention_days = 365 +# retention_days = 365 # forwarded to the runtime; also expires the adapter's memory audit # [memory.repos."acme/billing"] # enabled = true # per-repo opt-in diff --git a/crates/server/src/main.rs b/crates/server/src/main.rs index 20a8c92..00dd1b0 100644 --- a/crates/server/src/main.rs +++ b/crates/server/src/main.rs @@ -111,6 +111,27 @@ async fn main() -> Result<()> { worker::run(worker_config, worker_store, worker_notify).await; }); + // Memory retention sweep (issue #6): when a retention horizon is + // configured, periodically expire audit rows older than it. The + // first tick fires immediately, so a stale audit is trimmed at boot. + if let Some(retention_days) = config.memory.retention_days { + let sweep_store = store.clone(); + tokio::spawn(async move { + let mut ticker = + tokio::time::interval(std::time::Duration::from_secs(6 * 3600)); + loop { + ticker.tick().await; + match sweep_store.expire_memory_activity(retention_days).await { + Ok(0) => {} + Ok(expired) => { + tracing::info!(expired, "expired memory activity past retention") + } + Err(e) => tracing::error!("memory retention sweep failed: {e:#}"), + } + } + }); + } + // Build router. let state = AppState { config: config.clone(), diff --git a/crates/store/src/lib.rs b/crates/store/src/lib.rs index 352fdef..9df1fe3 100644 --- a/crates/store/src/lib.rs +++ b/crates/store/src/lib.rs @@ -608,12 +608,19 @@ impl Store { let mut conn = conn.lock().expect("store mutex poisoned"); let tx = conn.transaction()?; for row in &rows { + // Honor an explicit timestamp when provided (tests/backfills); + // otherwise stamp the current time. + let at = if row.at.is_empty() { + now_rfc3339() + } else { + row.at.clone() + }; tx.execute( "INSERT INTO memory_activity (at, installation_id, repo, task_id, op, target, scope, outcome) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8)", params![ - now_rfc3339(), + at, row.installation_id, row.repo, row.task_id, @@ -739,6 +746,30 @@ impl Store { .await .expect("store task panicked") } + + /// Deletes memory-activity audit rows older than `cutoff` (RFC 3339) — + /// retention expiry (issue #6). Revocations are active policy and are never + /// expired: an expired revocation would silently un-revoke memory. + pub async fn expire_memory_activity_before(&self, cutoff: &str) -> Result { + let conn = self.conn.clone(); + let cutoff = cutoff.to_string(); + tokio::task::spawn_blocking(move || { + let conn = conn.lock().expect("store mutex poisoned"); + let n = conn.execute( + "DELETE FROM memory_activity WHERE at < ?1", + params![cutoff], + )?; + Ok(n) + }) + .await + .expect("store task panicked") + } + + /// Deletes memory-activity audit rows older than `retention_days` from now. + pub async fn expire_memory_activity(&self, retention_days: u32) -> Result { + let cutoff = (chrono::Utc::now() - chrono::Duration::days(retention_days as i64)).to_rfc3339(); + self.expire_memory_activity_before(&cutoff).await + } } /// One recorded memory operation for the inspect/audit trail (issue #6). @@ -1099,6 +1130,36 @@ mod tests { assert!(store.revocations_for(2, "OpenCoven/billing").await.unwrap().is_empty()); } + #[tokio::test] + async fn retention_expiry_removes_old_activity_but_keeps_revocations() { + let store = Store::open_in_memory().unwrap(); + // An old audit row (explicit timestamp) and a fresh one (empty → now). + let mut old = memory_row(1, "OpenCoven/a", "repo/OpenCoven/a/old", "accepted"); + old.at = "2020-01-01T00:00:00+00:00".to_string(); + store.record_memory_activity(vec![old]).await.unwrap(); + store + .record_memory_activity(vec![memory_row(1, "OpenCoven/a", "repo/OpenCoven/a/new", "accepted")]) + .await + .unwrap(); + // A revocation must survive the sweep. + store.record_revocation(1, "OpenCoven/a", "repo/OpenCoven/a/x").await.unwrap(); + + let removed = store + .expire_memory_activity_before("2021-01-01T00:00:00+00:00") + .await + .unwrap(); + assert_eq!(removed, 1, "only the 2020 row expires"); + + let remaining = store.list_memory(None).await.unwrap(); + assert_eq!(remaining.len(), 1); + assert_eq!(remaining[0].target, "repo/OpenCoven/a/new"); + // Revocation is untouched — expiring it would un-revoke memory. + assert_eq!( + store.revocations_for(1, "OpenCoven/a").await.unwrap(), + vec!["repo/OpenCoven/a/x"] + ); + } + #[tokio::test] async fn delete_on_uninstall_purges_activity_and_revocations() { let store = Store::open_in_memory().unwrap(); diff --git a/docs/memory-contract.md b/docs/memory-contract.md index 6d4ab46..bb6de90 100644 --- a/docs/memory-contract.md +++ b/docs/memory-contract.md @@ -320,9 +320,9 @@ values are sane. prefix-addressable keys; `POST /api/github/memory/revoke` with adapter-side enforcement (revoked keys refused on future reads/writes) plus a denial list forwarded to the runtime; `delete_on_uninstall`; read citation on the review - surface. **Done**, except **retention expiry**, which needs a periodic sweep - (an operational enhancement, not a contract requirement) and remains a - follow-up. + surface; and **retention expiry** — a periodic server sweep drops audit rows + past `retention_days` (revocations are never expired), while the same horizon + is forwarded to the runtime for the memory bytes themselves. **Done.** The adapter side is complete. What is inherently bilateral — the runtime honoring the `denied` list and physically deleting revoked bytes — is the