From ee95701f7d9a2a7d4987bacf16e5644544e69ff0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=80=81=E8=92=8B?= <23394662@qq.com> Date: Wed, 8 Jul 2026 00:01:15 +0800 Subject: [PATCH 1/2] fix(memory): replace yaml_quote with sanitize_hint in consolidation frontmatter writers Replace yaml_quote() in fact.rs and quote-replacing sanitize() in episode.rs with sanitize_hint() that only replaces newlines and ASCII control chars with spaces, without YAML double-quoting or backslash escaping. The hand-rolled frontmatter readers (parse_frontmatter_flat / extract_title_and_description) use split_once(": ") + trim_matches('"') and do NOT interpret YAML escapes, so YAML-style escaping is asymmetric and causes a round-trip regression on Windows paths containing backslashes. This aligns with the standard established in PR 1154. Changes: - fact.rs: replace yaml_quote() with sanitize_hint() - episode.rs: update sanitize() to match sanitize_hint() pattern - Add 10 unit tests: 8 for sanitize_hint edge cases plus 2 for title/path with special chars in to_markdown() --- src/agent-memory/src/consolidation/episode.rs | 12 +- src/agent-memory/src/consolidation/fact.rs | 114 ++++++++++++++++-- 2 files changed, 114 insertions(+), 12 deletions(-) diff --git a/src/agent-memory/src/consolidation/episode.rs b/src/agent-memory/src/consolidation/episode.rs index d35e08754..1d9e55b8d 100644 --- a/src/agent-memory/src/consolidation/episode.rs +++ b/src/agent-memory/src/consolidation/episode.rs @@ -161,8 +161,18 @@ impl Episode { ) } + /// Sanitize a value for safe frontmatter inclusion. + /// Replaces newlines and ASCII control chars with spaces. + /// Does NOT apply YAML double-quoting or backslash escaping: + /// the hand-rolled frontmatter readers do not interpret those. fn sanitize(&self, s: &str) -> String { - s.replace('\n', " ").replace('"', "'") + s.chars() + .map(|c| match c { + '\n' | '\r' => ' ', + c if c.is_ascii_control() => ' ', + other => other, + }) + .collect() } } diff --git a/src/agent-memory/src/consolidation/fact.rs b/src/agent-memory/src/consolidation/fact.rs index 9817a4127..6c21e8679 100644 --- a/src/agent-memory/src/consolidation/fact.rs +++ b/src/agent-memory/src/consolidation/fact.rs @@ -106,12 +106,12 @@ impl ConsolidatedFact { out.push_str(&format!("id: {}\n", self.id)); out.push_str(&format!("session_id: {}\n", self.session_id)); out.push_str(&format!("category: {}\n", self.category)); - out.push_str(&format!("title: {}\n", yaml_quote(&self.title))); + out.push_str(&format!("title: {}\n", sanitize_hint(&self.title))); out.push_str(&format!("source_tool: {}\n", self.source_tool)); if !self.related_paths.is_empty() { out.push_str("related_paths:\n"); for p in &self.related_paths { - out.push_str(&format!(" - {}\n", yaml_quote(p))); + out.push_str(&format!(" - {}\n", sanitize_hint(p))); } } out.push_str(&format!("created_at: {}\n", self.created_at)); @@ -131,15 +131,27 @@ impl ConsolidatedFact { } } -/// Emit a YAML double-quoted scalar. Escapes `\`, `"`, and newlines so -/// user-controlled values (file paths, search queries, error messages) -/// cannot break frontmatter parsing. -fn yaml_quote(s: &str) -> String { - let escaped: String = s - .replace('\\', "\\\\") - .replace('"', "\\\"") - .replace('\n', " "); - format!("\"{escaped}\"") +/// Sanitize a value for safe frontmatter inclusion. +/// +/// The hand-rolled frontmatter readers (parse_frontmatter_flat / +/// extract_title_and_description) use split_once(": ") + +/// trim_matches('"') and do NOT interpret YAML escapes or double-quoting. +/// YAML-style escaping would be asymmetric and cause round-trip +/// regression on Windows paths containing backslashes. +/// +/// Strategy: +/// - Replace newlines (\n, \r) with spaces to prevent premature +/// termination of the "---" frontmatter block marker. +/// - Replace other ASCII control characters (\x00-\x1F) with spaces. +/// - Keep all other characters unchanged, including #, :, ", and \. +fn sanitize_hint(s: &str) -> String { + s.chars() + .map(|c| match c { + '\n' | '\r' => ' ', + c if c.is_ascii_control() => ' ', + other => other, + }) + .collect() } #[cfg(test)] @@ -202,4 +214,84 @@ mod tests { assert_eq!(FactCategory::WorkingContext.to_string(), "working-context"); assert_eq!(FactCategory::Lesson.to_string(), "lesson"); } + + #[test] + fn sanitize_hint_preserves_normal_text() { + let result = sanitize_hint("a simple title"); + assert_eq!(result, "a simple title"); + } + + #[test] + fn sanitize_hint_preserves_hashes() { + let result = sanitize_hint("fix #123 and #456"); + assert_eq!(result, "fix #123 and #456"); + } + + #[test] + fn sanitize_hint_preserves_colons() { + let result = sanitize_hint("note: has a colon"); + assert_eq!(result, "note: has a colon"); + } + + #[test] + fn sanitize_hint_preserves_quotes_and_backslashes() { + let result = sanitize_hint("she said \"hello\""); + assert_eq!(result, "she said \"hello\""); + let result2 = sanitize_hint("C:\\Users\\admin"); + assert_eq!(result2, "C:\\Users\\admin"); + } + + #[test] + fn sanitize_hint_replaces_newlines() { + let result = sanitize_hint("line1\nline2"); + assert_eq!(result, "line1 line2"); + } + + #[test] + fn sanitize_hint_crlf() { + let result = sanitize_hint("line1\r\nline2"); + assert_eq!(result, "line1 line2"); + } + + #[test] + fn sanitize_hint_empty() { + let result = sanitize_hint(""); + assert_eq!(result, ""); + } + + #[test] + fn sanitize_hint_control_chars() { + let result = sanitize_hint("pre\x00mid\x1Fpost"); + assert_eq!(result, "pre mid post"); + } + + #[test] + fn fact_title_with_special_chars() { + let f = ConsolidatedFact::new( + "sid", + FactCategory::Lesson, + "Use const\nnot var".into(), + "Details".into(), + "mem_edit".into(), + vec![], + 0.7, + ); + let md = f.to_markdown(); + assert!(md.contains("title: Use const not var")); + } + + #[test] + fn fact_path_with_backslashes() { + let f = ConsolidatedFact::new( + "sid", + FactCategory::WorkingContext, + "Work".into(), + "Details".into(), + "mem_write".into(), + vec!["C:\\Users\\admin\\file.txt".into()], + 0.5, + ); + let md = f.to_markdown(); + assert!(md.contains("C:\\Users\\admin\\file.txt")); + } } From 53b99e319358fec3051a55c069e8495fbfd2bdf8 Mon Sep 17 00:00:00 2001 From: ralf003 <23394662@qq.com> Date: Wed, 8 Jul 2026 23:14:06 +0800 Subject: [PATCH 2/2] test(memory): add round-trip verification for sanitize_hint Add frontmatter write->read round-trip assertions to fact_title_with_special_chars and fact_path_with_backslashes tests: - Extract title/paths from to_markdown() output and verify they survive the write->read cycle without escaping artifacts. - Verify no YAML double-quoting artifacts are present. - Addresses review feedback from shiloong on PR #1388. --- src/agent-memory/src/consolidation/fact.rs | 46 ++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/src/agent-memory/src/consolidation/fact.rs b/src/agent-memory/src/consolidation/fact.rs index 6c21e8679..5d62035ca 100644 --- a/src/agent-memory/src/consolidation/fact.rs +++ b/src/agent-memory/src/consolidation/fact.rs @@ -277,7 +277,27 @@ mod tests { 0.7, ); let md = f.to_markdown(); + // Verify the title is encoded without YAML quoting (round-trip compatible) assert!(md.contains("title: Use const not var")); + // Round-trip: extract title from frontmatter and confirm it matches sanitized input + let title_line = md + .lines() + .skip_while(|l| *l != "---") + .skip(1) // skip opening --- + .find(|l| l.starts_with("title: ")) + .expect("title field missing in frontmatter"); + let extracted_title = title_line + .strip_prefix("title: ") + .expect("title prefix missing"); + assert_eq!( + extracted_title, "Use const not var", + "round-trip: special chars survive write->read" + ); + // Verify no YAML double-quoting artifact + assert!( + !md.contains("title: \""), + "must not use YAML double-quoting" + ); } #[test] @@ -292,6 +312,32 @@ mod tests { 0.5, ); let md = f.to_markdown(); + // Verify paths are kept verbatim (no YAML double-quoting) assert!(md.contains("C:\\Users\\admin\\file.txt")); + // Round-trip: extract related_paths from frontmatter and confirm + let mut paths = Vec::new(); + let mut in_frontmatter = false; + for line in md.lines() { + if line == "---" { + if in_frontmatter { + break; + } + in_frontmatter = true; + continue; + } + if in_frontmatter && line.starts_with(" - ") { + paths.push(line.strip_prefix(" - ").unwrap().to_string()); + } + } + assert_eq!( + paths, + vec!["C:\\Users\\admin\\file.txt"], + "round-trip: Windows paths survive write->read without escaping artifacts" + ); + // Verify no YAML double-quoting artifact + assert!( + !md.contains("C:\\\"Users"), + "must not use YAML double-quoting on paths" + ); } }