diff --git a/docs/config-reference.md b/docs/config-reference.md index 6be4d6a..cb8f3a0 100644 --- a/docs/config-reference.md +++ b/docs/config-reference.md @@ -129,13 +129,17 @@ Notes: | Key | Default | Purpose | |---|---|---| | `backend` | `sqlite` | `sqlite`, `lucid`, `markdown`, `none` | -| `auto_save` | `true` | automatic persistence | +| `auto_save` | `true` | persist user-stated inputs only (assistant outputs are excluded) | | `embedding_provider` | `none` | `none`, `openai`, or custom endpoint | | `embedding_model` | `text-embedding-3-small` | embedding model ID, or `hint:` route | | `embedding_dimensions` | `1536` | expected vector size for selected embedding model | | `vector_weight` | `0.7` | hybrid ranking vector weight | | `keyword_weight` | `0.3` | hybrid ranking keyword weight | +Notes: + +- Memory context injection ignores legacy `assistant_resp*` auto-save keys to prevent old model-authored summaries from being treated as facts. + ## `[[model_routes]]` and `[[embedding_routes]]` Use route hints so integrations can keep stable names while model IDs evolve. diff --git a/src/agent/agent.rs b/src/agent/agent.rs index c85473b..8dad045 100644 --- a/src/agent/agent.rs +++ b/src/agent/agent.rs @@ -10,7 +10,6 @@ use crate::providers::{self, ChatMessage, ChatRequest, ConversationMessage, Prov use crate::runtime; use crate::security::SecurityPolicy; use crate::tools::{self, Tool, ToolSpec}; -use crate::util::truncate_with_ellipsis; use anyhow::Result; use std::io::Write as IoWrite; use std::sync::Arc; @@ -487,14 +486,6 @@ impl Agent { ))); self.trim_history(); - if self.auto_save { - let summary = truncate_with_ellipsis(&final_text, 100); - let _ = self - .memory - .store("assistant_resp", &summary, MemoryCategory::Daily, None) - .await; - } - return Ok(final_text); } diff --git a/src/agent/loop_.rs b/src/agent/loop_.rs index 5d938fe..f5402b0 100644 --- a/src/agent/loop_.rs +++ b/src/agent/loop_.rs @@ -226,9 +226,16 @@ async fn build_context(mem: &dyn Memory, user_msg: &str, min_relevance_score: f6 if !relevant.is_empty() { context.push_str("[Memory context]\n"); for entry in &relevant { + if memory::is_assistant_autosave_key(&entry.key) { + continue; + } let _ = writeln!(context, "- {}: {}", entry.key, entry.content); } - context.push('\n'); + if context != "[Memory context]\n" { + context.push('\n'); + } else { + context.clear(); + } } } @@ -1433,15 +1440,6 @@ pub async fn run( final_output = response.clone(); println!("{response}"); observer.record_event(&ObserverEvent::TurnComplete); - - // Auto-save assistant response to daily log - if config.memory.auto_save { - let summary = truncate_with_ellipsis(&response, 100); - let response_key = autosave_memory_key("assistant_resp"); - let _ = mem - .store(&response_key, &summary, MemoryCategory::Daily, None) - .await; - } } else { println!("🦀 ZeroClaw Interactive Mode"); println!("Type /help for commands.\n"); @@ -1591,14 +1589,6 @@ pub async fn run( // Hard cap as a safety net. trim_history(&mut history, config.agent.max_history_messages); - - if config.memory.auto_save { - let summary = truncate_with_ellipsis(&response, 100); - let response_key = autosave_memory_key("assistant_resp"); - let _ = mem - .store(&response_key, &summary, MemoryCategory::Daily, None) - .await; - } } } @@ -2441,6 +2431,33 @@ Done."#; assert!(recalled.iter().any(|entry| entry.content.contains("45"))); } + #[tokio::test] + async fn build_context_ignores_legacy_assistant_autosave_entries() { + let tmp = TempDir::new().unwrap(); + let mem = SqliteMemory::new(tmp.path()).unwrap(); + mem.store( + "assistant_resp_poisoned", + "User suffered a fabricated event", + MemoryCategory::Daily, + None, + ) + .await + .unwrap(); + mem.store( + "user_msg_real", + "User asked for concise status updates", + MemoryCategory::Conversation, + None, + ) + .await + .unwrap(); + + let context = build_context(&mem, "status updates", 0.0).await; + assert!(context.contains("user_msg_real")); + assert!(!context.contains("assistant_resp_poisoned")); + assert!(!context.contains("fabricated event")); + } + // ═══════════════════════════════════════════════════════════════════════ // Recovery Tests - Tool Call Parsing Edge Cases // ═══════════════════════════════════════════════════════════════════════ diff --git a/src/agent/memory_loader.rs b/src/agent/memory_loader.rs index b171eed..bb7bfb5 100644 --- a/src/agent/memory_loader.rs +++ b/src/agent/memory_loader.rs @@ -1,4 +1,4 @@ -use crate::memory::Memory; +use crate::memory::{self, Memory}; use async_trait::async_trait; use std::fmt::Write; @@ -45,6 +45,9 @@ impl MemoryLoader for DefaultMemoryLoader { let mut context = String::from("[Memory context]\n"); for entry in entries { + if memory::is_assistant_autosave_key(&entry.key) { + continue; + } if let Some(score) = entry.score { if score < self.min_relevance_score { continue; @@ -67,8 +70,12 @@ impl MemoryLoader for DefaultMemoryLoader { mod tests { use super::*; use crate::memory::{Memory, MemoryCategory, MemoryEntry}; + use std::sync::Arc; struct MockMemory; + struct MockMemoryWithEntries { + entries: Arc>, + } #[async_trait] impl Memory for MockMemory { @@ -131,6 +138,56 @@ mod tests { } } + #[async_trait] + impl Memory for MockMemoryWithEntries { + async fn store( + &self, + _key: &str, + _content: &str, + _category: MemoryCategory, + _session_id: Option<&str>, + ) -> anyhow::Result<()> { + Ok(()) + } + + async fn recall( + &self, + _query: &str, + _limit: usize, + _session_id: Option<&str>, + ) -> anyhow::Result> { + Ok(self.entries.as_ref().clone()) + } + + async fn get(&self, _key: &str) -> anyhow::Result> { + Ok(None) + } + + async fn list( + &self, + _category: Option<&MemoryCategory>, + _session_id: Option<&str>, + ) -> anyhow::Result> { + Ok(vec![]) + } + + async fn forget(&self, _key: &str) -> anyhow::Result { + Ok(true) + } + + async fn count(&self) -> anyhow::Result { + Ok(self.entries.len()) + } + + async fn health_check(&self) -> bool { + true + } + + fn name(&self) -> &str { + "mock-with-entries" + } + } + #[tokio::test] async fn default_loader_formats_context() { let loader = DefaultMemoryLoader::default(); @@ -138,4 +195,36 @@ mod tests { assert!(context.contains("[Memory context]")); assert!(context.contains("- k: v")); } + + #[tokio::test] + async fn default_loader_skips_legacy_assistant_autosave_entries() { + let loader = DefaultMemoryLoader::new(5, 0.0); + let memory = MockMemoryWithEntries { + entries: Arc::new(vec![ + MemoryEntry { + id: "1".into(), + key: "assistant_resp_legacy".into(), + content: "fabricated detail".into(), + category: MemoryCategory::Daily, + timestamp: "now".into(), + session_id: None, + score: Some(0.95), + }, + MemoryEntry { + id: "2".into(), + key: "user_fact".into(), + content: "User prefers concise answers".into(), + category: MemoryCategory::Conversation, + timestamp: "now".into(), + session_id: None, + score: Some(0.9), + }, + ]), + }; + + let context = loader.load_context(&memory, "answer style").await.unwrap(); + assert!(context.contains("user_fact")); + assert!(!context.contains("assistant_resp_legacy")); + assert!(!context.contains("fabricated detail")); + } } diff --git a/src/agent/tests.rs b/src/agent/tests.rs index fd73eb1..356987e 100644 --- a/src/agent/tests.rs +++ b/src/agent/tests.rs @@ -624,7 +624,7 @@ async fn history_trims_after_max_messages() { // ═══════════════════════════════════════════════════════════════════════════ #[tokio::test] -async fn auto_save_stores_messages_in_memory() { +async fn auto_save_stores_only_user_messages_in_memory() { let (mem, _tmp) = make_sqlite_memory(); let provider = Box::new(ScriptedProvider::new(vec![text_response( "I remember everything", @@ -639,11 +639,25 @@ async fn auto_save_stores_messages_in_memory() { let _ = agent.turn("Remember this fact").await.unwrap(); - // Both user message and assistant response should be saved + // Auto-save only persists user-stated input, never assistant-generated summaries. let count = mem.count().await.unwrap(); + assert_eq!( + count, 1, + "Expected exactly 1 user memory entry, got {count}" + ); + + let stored = mem.get("user_msg").await.unwrap(); + assert!(stored.is_some(), "Expected user_msg key to be present"); + assert_eq!( + stored.unwrap().content, + "Remember this fact", + "Stored memory should match the original user message" + ); + + let assistant = mem.get("assistant_resp").await.unwrap(); assert!( - count >= 2, - "Expected at least 2 memory entries, got {count}" + assistant.is_none(), + "assistant_resp should not be auto-saved anymore" ); } diff --git a/src/channels/mod.rs b/src/channels/mod.rs index 40361b3..70919ff 100644 --- a/src/channels/mod.rs +++ b/src/channels/mod.rs @@ -293,6 +293,10 @@ fn compact_sender_history(ctx: &ChannelRuntimeContext, sender_key: &str) -> bool } fn should_skip_memory_context_entry(key: &str, content: &str) -> bool { + if memory::is_assistant_autosave_key(key) { + return true; + } + if key.trim().to_ascii_lowercase().ends_with("_history") { return true; } @@ -2167,6 +2171,10 @@ mod tests { "telegram_123_history", r#"[{"role":"user"}]"# )); + assert!(should_skip_memory_context_entry( + "assistant_resp_legacy", + "fabricated memory" + )); assert!(!should_skip_memory_context_entry("telegram_123_45", "hi")); } diff --git a/src/config/schema.rs b/src/config/schema.rs index 011c329..dbd84f0 100644 --- a/src/config/schema.rs +++ b/src/config/schema.rs @@ -1387,7 +1387,7 @@ pub struct MemoryConfig { /// /// `postgres` requires `[storage.provider.config]` with `db_url` (`dbURL` alias supported). pub backend: String, - /// Auto-save conversation context to memory + /// Auto-save user-stated conversation input to memory (assistant output is excluded) pub auto_save: bool, /// Run memory/session hygiene (archiving + retention cleanup) #[serde(default = "default_hygiene_enabled")] diff --git a/src/memory/mod.rs b/src/memory/mod.rs index dd9f0d1..a9d3cca 100644 --- a/src/memory/mod.rs +++ b/src/memory/mod.rs @@ -75,6 +75,13 @@ pub fn effective_memory_backend_name( memory_backend.trim().to_ascii_lowercase() } +/// Legacy auto-save key used for model-authored assistant summaries. +/// These entries are treated as untrusted context and should not be re-injected. +pub fn is_assistant_autosave_key(key: &str) -> bool { + let normalized = key.trim().to_ascii_lowercase(); + normalized == "assistant_resp" || normalized.starts_with("assistant_resp_") +} + #[derive(Debug, Clone, PartialEq, Eq)] struct ResolvedEmbeddingConfig { provider: String, @@ -343,6 +350,15 @@ mod tests { assert_eq!(mem.name(), "sqlite"); } + #[test] + fn assistant_autosave_key_detection_matches_legacy_patterns() { + assert!(is_assistant_autosave_key("assistant_resp")); + assert!(is_assistant_autosave_key("assistant_resp_1234")); + assert!(is_assistant_autosave_key("ASSISTANT_RESP_abcd")); + assert!(!is_assistant_autosave_key("assistant_response")); + assert!(!is_assistant_autosave_key("user_msg_1234")); + } + #[test] fn factory_markdown() { let tmp = TempDir::new().unwrap();