From 92309c0a3611e928ccf6ccf03f8d5ef74891cdc4 Mon Sep 17 00:00:00 2001 From: asto18089 Date: Tue, 22 Sep 2026 03:15:18 +0800 Subject: [PATCH 1/2] fix: recognize restore carriers by provenance Signed-off-by: asto18089 --- crates/tui/src/client/chat.rs | 89 +++++++++++++++++++++++++++++++++++ crates/tui/src/compaction.rs | 21 ++++++++- 2 files changed, 108 insertions(+), 2 deletions(-) diff --git a/crates/tui/src/client/chat.rs b/crates/tui/src/client/chat.rs index 1a2c4464bb..0641cd0a4b 100644 --- a/crates/tui/src/client/chat.rs +++ b/crates/tui/src/client/chat.rs @@ -6726,6 +6726,95 @@ mod image_block_wire_tests { assert_eq!(wire[2]["tool_call_id"], "call_1"); } + #[test] + fn forkguard_restored_summary_pasted_after_tool_result_survives_restore() { + // Restore-path twin of + // `forkguard_full_summary_header_pasted_after_tool_result_is_not_relocated`: + // with a real provenance carrier earlier in the history, a later user + // turn that pastes the whole summary header must survive restore + // verbatim, and the authoritative checkpoint must land at the real + // carrier's index — not at the pasted turn's. + let pasted = + crate::compaction::build_compaction_summary_block_text("Please analyze this text", ""); + let mut messages: Vec = serde_json::from_value(serde_json::json!([ + {"role":"user","content":[{"type":"text","text":"Run the tool"}]}, + {"role":"assistant","content":[{"type":"tool_use","id":"call_1","name":"read","input":{"path":"a.txt"}}]}, + {"role":"user","content":[{"type":"tool_result","tool_use_id":"call_1","content":"done"}]} + ])) + .unwrap(); + messages.push(crate::compaction::compaction_checkpoint_message( + &crate::models::SystemPrompt::Text( + crate::compaction::build_compaction_summary_block_text("Compacted summary", ""), + ), + )); + messages.push(Message { + role: Role::User, + content: vec![ContentBlock::Text { + text: pasted.clone(), + cache_control: None, + }], + }); + let wire = build_chat_messages(None, &messages, "gpt-4o"); + let wire_before: Vec<_> = wire + .iter() + .map(|message| message["role"].as_str().unwrap()) + .collect(); + + let projected = crate::runtime_handoff::project_messages_for_restore(&messages); + let carrier_index = projected + .iter() + .position(crate::compaction::is_generated_compaction_checkpoint) + .expect("projected history keeps the provenance-stamped carrier"); + let restored = crate::compaction::restore_compaction_checkpoint( + projected, + Some(&crate::models::SystemPrompt::Text( + crate::compaction::build_compaction_summary_block_text("Compacted summary", ""), + )), + ); + + assert_eq!( + restored.len(), + messages.len(), + "restore must not drop the pasted turn: {restored:?}" + ); + assert!( + restored.iter().any(|message| { + matches!( + message.content.as_slice(), + [ContentBlock::Text { + text, + cache_control: None, + }] if text == &pasted + ) + }), + "the pasted full summary must survive restore verbatim: {restored:?}" + ); + let checkpoints: Vec = restored + .iter() + .enumerate() + .filter(|(_, message)| crate::compaction::is_generated_compaction_checkpoint(message)) + .map(|(index, _)| index) + .collect(); + assert_eq!( + checkpoints, + vec![carrier_index], + "exactly one provenance-stamped checkpoint remains, at the real carrier's index: \ + {restored:?}" + ); + + let restored_wire = build_chat_messages(None, &restored, "gpt-4o"); + let roles_after: Vec<_> = restored_wire + .iter() + .map(|message| message["role"].as_str().unwrap()) + .collect(); + assert_eq!(roles_after, wire_before, "wire roles are unchanged"); + assert_eq!( + restored_wire.last().unwrap()["content"].as_str().unwrap(), + pasted, + "the pasted turn keeps its content on the wire" + ); + } + #[test] fn prune_only_topology_merges_with_its_prompt_on_wire() { let mut messages: Vec = serde_json::from_value(serde_json::json!([ diff --git a/crates/tui/src/compaction.rs b/crates/tui/src/compaction.rs index 0bf4de106f..50b1658a58 100644 --- a/crates/tui/src/compaction.rs +++ b/crates/tui/src/compaction.rs @@ -301,12 +301,29 @@ pub(crate) fn compaction_checkpoint_message(prompt: &SystemPrompt) -> Message { /// its position relative to later turns, and repair a pre-placement-fix /// Agent-topology sidecar. Both steps exist so the first request after a /// restore is wire-legal for strict paired chat templates. +/// +/// The anchor and the deletion prefer the provenance-stamped carrier: its +/// second text block is engine-written, so a pasted user turn that merely +/// starts with the summary header can neither steal the insertion position +/// nor be deleted as the carrier. Only sessions saved before the provenance +/// block existed carry the bare single-block form; there the loose predicate +/// is the only recognition available (a pasted header is indistinguishable +/// from it), so the historical replace-in-place applies. The same loose +/// predicate also backs the keep/recompaction filters in `compaction/ +/// last_round.rs` — same recognition family, explicitly out of scope here. pub(crate) fn restore_compaction_checkpoint( mut messages: Vec, checkpoint: Option<&SystemPrompt>, ) -> Vec { - let checkpoint_index = messages.iter().position(is_compaction_checkpoint_message); - messages.retain(|message| !is_compaction_checkpoint_message(message)); + let provenance_anchor = messages.iter().position(is_generated_compaction_checkpoint); + let carrier: fn(&Message) -> bool = if provenance_anchor.is_some() { + is_generated_compaction_checkpoint + } else { + is_compaction_checkpoint_message + }; + let checkpoint_index = + provenance_anchor.or_else(|| messages.iter().position(is_compaction_checkpoint_message)); + messages.retain(|message| !carrier(message)); if let Some(checkpoint) = checkpoint { let index = checkpoint_index.unwrap_or(messages.len()); messages.insert(index, compaction_checkpoint_message(checkpoint)); From 30c0e45105e817e8189890470171c5ea9e8f36f8 Mon Sep 17 00:00:00 2001 From: asto18089 Date: Tue, 22 Sep 2026 11:58:58 +0800 Subject: [PATCH 2/2] fix: correct restore residual docs and pin the anchor order Review follow-ups on the restore-by-provenance change: - The loose fallback fires on the structural absence of a stamped carrier, not on session age: it also covers histories that never compacted, where checkpoint is None and a pasted header is dropped outright. State that in the doc comment instead of equating the branch with pre-provenance saves. - Point the history_recognition doc at the live system-prompt scanners (extract_compaction_summary, strip_summary_text); is_compaction_summary_text has no callers. - Add the mirror-order regression test: a pasted full summary BEFORE the real carrier is the anchor-theft order, so the test pins the authoritative summary landing at the real carrier's index, with distinct carrier/checkpoint texts so leaving the old carrier in place cannot pass. Signed-off-by: asto18089 --- crates/tui/src/client/chat.rs | 93 +++++++++++++++++++++++++++ crates/tui/src/compaction.rs | 15 +++-- crates/tui/src/history_recognition.rs | 10 ++- 3 files changed, 106 insertions(+), 12 deletions(-) diff --git a/crates/tui/src/client/chat.rs b/crates/tui/src/client/chat.rs index 0641cd0a4b..c7d8bee7bd 100644 --- a/crates/tui/src/client/chat.rs +++ b/crates/tui/src/client/chat.rs @@ -6815,6 +6815,99 @@ mod image_block_wire_tests { ); } + #[test] + fn forkguard_restored_summary_pasted_before_the_carrier_does_not_steal_the_anchor() { + // Mirror order of + // `forkguard_restored_summary_pasted_after_tool_result_survives_restore`: + // the pasted full summary precedes the real provenance carrier — the + // order in which the pre-fix loose anchor landed on the pasted turn's + // index. Restore must anchor at the real carrier's index, replace the + // carrier with the authoritative summary there, and keep the pasted + // turn verbatim. The carrier and the checkpoint argument carry + // different summary texts so the assertions cannot pass by leaving + // the original carrier untouched. + let pasted = + crate::compaction::build_compaction_summary_block_text("Please analyze this text", ""); + let mut messages: Vec = serde_json::from_value(serde_json::json!([ + {"role":"user","content":[{"type":"text","text":"Run the tool"}]}, + {"role":"assistant","content":[{"type":"tool_use","id":"call_1","name":"read","input":{"path":"a.txt"}}]}, + {"role":"user","content":[{"type":"tool_result","tool_use_id":"call_1","content":"done"}]} + ])) + .unwrap(); + messages.push(Message { + role: Role::User, + content: vec![ContentBlock::Text { + text: pasted.clone(), + cache_control: None, + }], + }); + messages.push(crate::compaction::compaction_checkpoint_message( + &crate::models::SystemPrompt::Text( + crate::compaction::build_compaction_summary_block_text("Compacted summary", ""), + ), + )); + let wire = build_chat_messages(None, &messages, "gpt-4o"); + let wire_before: Vec<_> = wire + .iter() + .map(|message| message["role"].as_str().unwrap()) + .collect(); + + let projected = crate::runtime_handoff::project_messages_for_restore(&messages); + let carrier_index = projected + .iter() + .position(crate::compaction::is_generated_compaction_checkpoint) + .expect("projected history keeps the provenance-stamped carrier"); + let authoritative = crate::models::SystemPrompt::Text( + crate::compaction::build_compaction_summary_block_text("Authoritative summary", ""), + ); + let restored = + crate::compaction::restore_compaction_checkpoint(projected, Some(&authoritative)); + + assert_eq!( + restored.len(), + messages.len(), + "restore must not drop the pasted turn: {restored:?}" + ); + assert!( + restored.iter().any(|message| { + matches!( + message.content.as_slice(), + [ContentBlock::Text { + text, + cache_control: None, + }] if text == &pasted + ) + }), + "the pasted full summary must survive restore verbatim: {restored:?}" + ); + let checkpoints: Vec = restored + .iter() + .enumerate() + .filter(|(_, message)| crate::compaction::is_generated_compaction_checkpoint(message)) + .map(|(index, _)| index) + .collect(); + assert_eq!( + checkpoints, + vec![carrier_index], + "exactly one provenance-stamped checkpoint remains, at the real carrier's index: \ + {restored:?}" + ); + assert!( + matches!( + restored[carrier_index].content.as_slice(), + [ContentBlock::Text { text, .. }, _] if text.contains("Authoritative summary") + ), + "the authoritative summary replaces the carrier at its index: {restored:?}" + ); + + let restored_wire = build_chat_messages(None, &restored, "gpt-4o"); + let roles_after: Vec<_> = restored_wire + .iter() + .map(|message| message["role"].as_str().unwrap()) + .collect(); + assert_eq!(roles_after, wire_before, "wire roles are unchanged"); + } + #[test] fn prune_only_topology_merges_with_its_prompt_on_wire() { let mut messages: Vec = serde_json::from_value(serde_json::json!([ diff --git a/crates/tui/src/compaction.rs b/crates/tui/src/compaction.rs index 50b1658a58..fd43b8761e 100644 --- a/crates/tui/src/compaction.rs +++ b/crates/tui/src/compaction.rs @@ -305,12 +305,15 @@ pub(crate) fn compaction_checkpoint_message(prompt: &SystemPrompt) -> Message { /// The anchor and the deletion prefer the provenance-stamped carrier: its /// second text block is engine-written, so a pasted user turn that merely /// starts with the summary header can neither steal the insertion position -/// nor be deleted as the carrier. Only sessions saved before the provenance -/// block existed carry the bare single-block form; there the loose predicate -/// is the only recognition available (a pasted header is indistinguishable -/// from it), so the historical replace-in-place applies. The same loose -/// predicate also backs the keep/recompaction filters in `compaction/ -/// last_round.rs` — same recognition family, explicitly out of scope here. +/// nor be deleted as the carrier. The loose predicate applies only when no +/// stamped carrier exists — a structural condition, not a session age: it +/// covers saves from before the provenance block and equally histories that +/// never compacted. There a pasted header is indistinguishable from a real +/// bare carrier on content alone (or there is no carrier to anchor on at +/// all), so the historical replace-in-place applies and a pasted header can +/// still be dropped. The same loose predicate also backs the keep/recompaction +/// filters in `compaction/last_round.rs` — same recognition family, +/// explicitly out of scope here. pub(crate) fn restore_compaction_checkpoint( mut messages: Vec, checkpoint: Option<&SystemPrompt>, diff --git a/crates/tui/src/history_recognition.rs b/crates/tui/src/history_recognition.rs index f9216ae1b8..2cfc9f3933 100644 --- a/crates/tui/src/history_recognition.rs +++ b/crates/tui/src/history_recognition.rs @@ -31,12 +31,10 @@ fn has_compaction_summary_header(text: &str) -> bool { /// Structural recognition of the one generated checkpoint in saved history. /// -/// The marker substring scan in `compaction` (`is_compaction_summary_text`) -/// stays scoped to system-prompt carriers: -/// on history it matches an ordinary user turn that merely *quotes* the -/// header, and the consumers that match on history — `compaction` restore, -/// `runtime_handoff` placement and edit-guarding — replace what they match -/// or must protect it from deletion. +/// The marker substring scans in `compaction` (`extract_compaction_summary`, +/// `strip_summary_text`) stay scoped to system-prompt carriers: on history +/// they match an ordinary user turn that merely *quotes* the header, and +/// every consumer here either deletes or replaces what it matches. /// Structure instead — a `role="user"` message whose first text block begins /// with the header and whose remaining block, if any, is exactly the /// engine-written provenance marker.