From e86b372fa60b352eaff98b487ee1023d6a87592a Mon Sep 17 00:00:00 2001 From: pikaxinge <68273313+pikaxinge@users.noreply.github.com> Date: Tue, 10 Mar 2026 01:13:13 +0800 Subject: [PATCH] fix: prevent irreversible context loss when compaction archive write fails (#754) * fix(compaction): preserve turns when archival write fails * style: cargo fmt Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: Zaki Co-authored-by: Claude Opus 4.6 --- src/agent/compaction.rs | 133 ++++++++++++++++++++++++++++++++-------- 1 file changed, 108 insertions(+), 25 deletions(-) diff --git a/src/agent/compaction.rs b/src/agent/compaction.rs index 46980c79..583d92de 100644 --- a/src/agent/compaction.rs +++ b/src/agent/compaction.rs @@ -103,27 +103,26 @@ impl ContextCompactor { // Generate summary let summary = self.generate_summary(&to_summarize).await?; - // Write to workspace if available - let summary_written = if let Some(ws) = workspace { + // Write to workspace if available. + // If archival fails, preserve turns to avoid context loss. + let (summary_written, turns_removed) = if let Some(ws) = workspace { match self.write_summary_to_workspace(ws, &summary).await { - Ok(()) => true, + Ok(()) => { + thread.truncate_turns(keep_recent); + (true, turns_to_remove) + } Err(e) => { - tracing::warn!( - "Compaction summary write failed (turns will still be truncated): {}", - e - ); - false + tracing::warn!("Compaction summary write failed (turns preserved): {}", e); + (false, 0) } } } else { - false + thread.truncate_turns(keep_recent); + (false, turns_to_remove) }; - // Truncate thread - thread.truncate_turns(keep_recent); - Ok(CompactionPartial { - turns_removed: turns_to_remove, + turns_removed, summary_written, summary: Some(summary), }) @@ -165,23 +164,20 @@ impl ContextCompactor { // Format turns for storage let content = format_turns_for_storage(old_turns); - // Write to workspace - let written = match self.write_context_to_workspace(ws, &content).await { - Ok(()) => true, + // Write to workspace. If archival fails, preserve turns. + let (written, turns_removed) = match self.write_context_to_workspace(ws, &content).await { + Ok(()) => { + thread.truncate_turns(keep_recent); + (true, turns_to_remove) + } Err(e) => { - tracing::warn!( - "Compaction context write failed (turns will still be truncated): {}", - e - ); - false + tracing::warn!("Compaction context write failed (turns preserved): {}", e); + (false, 0) } }; - // Truncate - thread.truncate_turns(keep_recent); - Ok(CompactionPartial { - turns_removed: turns_to_remove, + turns_removed, summary_written: written, summary: None, }) @@ -362,6 +358,19 @@ mod tests { thread } + #[cfg(feature = "libsql")] + async fn make_unmigrated_workspace() -> crate::workspace::Workspace { + use crate::db::Database; + use crate::db::libsql::LibSqlBackend; + + // Intentionally skip migrations so workspace append operations fail. + let backend = LibSqlBackend::new_memory() + .await + .expect("should create in-memory libsql backend"); + let db: Arc = Arc::new(backend); + crate::workspace::Workspace::new_with_db("compaction-test", db) + } + // ------------------------------------------------------------------ // 1. compact_truncate keeps last N turns // ------------------------------------------------------------------ @@ -560,6 +569,43 @@ mod tests { assert_eq!(llm.calls(), 0); } + #[cfg(feature = "libsql")] + #[tokio::test] + async fn test_compact_with_summary_preserves_turns_when_workspace_write_fails() { + let llm = Arc::new(StubLlm::new("summary")); + let compactor = make_compactor(llm.clone()); + let mut thread = make_thread(8); + let original_inputs: Vec = + thread.turns.iter().map(|t| t.user_input.clone()).collect(); + let workspace = make_unmigrated_workspace().await; + + let result = compactor + .compact( + &mut thread, + CompactionStrategy::Summarize { keep_recent: 3 }, + Some(&workspace), + ) + .await + .expect("compact should succeed even when workspace write fails"); + + // On archival failure, no turns should be removed. + assert_eq!(thread.turns.len(), 8); + assert_eq!( + thread + .turns + .iter() + .map(|t| t.user_input.as_str()) + .collect::>(), + original_inputs + .iter() + .map(|s| s.as_str()) + .collect::>() + ); + assert_eq!(result.turns_removed, 0); + assert!(!result.summary_written); + assert_eq!(llm.calls(), 1); + } + // ------------------------------------------------------------------ // 7. compact_to_workspace without workspace falls back to truncation // ------------------------------------------------------------------ @@ -608,6 +654,43 @@ mod tests { assert_eq!(result.turns_removed, 0); } + #[cfg(feature = "libsql")] + #[tokio::test] + async fn test_compact_to_workspace_preserves_turns_when_workspace_write_fails() { + let llm = Arc::new(StubLlm::new("unused")); + let compactor = make_compactor(llm.clone()); + let mut thread = make_thread(20); + let original_inputs: Vec = + thread.turns.iter().map(|t| t.user_input.clone()).collect(); + let workspace = make_unmigrated_workspace().await; + + let result = compactor + .compact( + &mut thread, + CompactionStrategy::MoveToWorkspace, + Some(&workspace), + ) + .await + .expect("compact should succeed even when workspace write fails"); + + // On archival failure, no turns should be removed. + assert_eq!(thread.turns.len(), 20); + assert_eq!( + thread + .turns + .iter() + .map(|t| t.user_input.as_str()) + .collect::>(), + original_inputs + .iter() + .map(|s| s.as_str()) + .collect::>() + ); + assert_eq!(result.turns_removed, 0); + assert!(!result.summary_written); + assert_eq!(llm.calls(), 0); + } + // ------------------------------------------------------------------ // 9. format_turns_for_storage includes tool calls // ------------------------------------------------------------------