From 285b7aa216444697fd6c92c83054a31c6e1d2f3c Mon Sep 17 00:00:00 2001 From: z27014 Date: Wed, 16 Sep 2026 15:00:13 +0800 Subject: [PATCH 1/7] fix: preserve tool rounds after compaction Signed-off-by: z27014 --- crates/tui/src/runtime_handoff.rs | 75 ++++++++++++++++++++++++++++++- 1 file changed, 73 insertions(+), 2 deletions(-) diff --git a/crates/tui/src/runtime_handoff.rs b/crates/tui/src/runtime_handoff.rs index cd22d031cc..daa0919ced 100644 --- a/crates/tui/src/runtime_handoff.rs +++ b/crates/tui/src/runtime_handoff.rs @@ -456,13 +456,45 @@ fn is_agent_topology_checkpoint(message: &Message) -> bool { /// compaction. A current empty topology is still meaningful: it overrides a /// narrative summary or old runtime event that says an Agent remains live. /// Replays are idempotent because the previous sidecar is structurally removed -/// before the replacement is appended. +/// before the replacement is inserted. At a tool-result boundary, place it +/// after the latest user input, before that round's assistant/tool chain. +/// Strict paired chat templates cannot encode a tool result followed by a +/// user checkpoint. Compaction already replaces history; this does not mutate +/// the session-pinned system/cache prefix or impersonate an assistant reply. pub(crate) fn replace_agent_topology_checkpoint( messages: &mut Vec, snapshots: &[SubAgentResult], ) { messages.retain(|message| !is_agent_topology_checkpoint(message)); - messages.push(agent_topology_checkpoint_message(snapshots)); + let ends_with_tool_result = messages.last().is_some_and(|message| { + message.content.iter().any(|block| { + matches!( + block, + ContentBlock::ToolResult { .. } + | ContentBlock::ToolSearchToolResult { .. } + | ContentBlock::CodeExecutionToolResult { .. } + ) + }) + }); + let position = if ends_with_tool_result { + messages + .iter() + .rposition(|message| { + classify_user_turn_prompt(message) != UserTurnPromptKind::NotPrompt + }) + .map_or_else( + || { + messages + .iter() + .position(|message| message.role.is_assistant_like()) + .unwrap_or(messages.len()) + }, + |index| index + 1, + ) + } else { + messages.len() + }; + messages.insert(position, agent_topology_checkpoint_message(snapshots)); } #[cfg(test)] @@ -1237,6 +1269,45 @@ mod tests { assert_eq!(project_messages_for_restore(&projected), projected); } + #[test] + fn forkguard_compaction_topology_preserves_tool_round_boundary() { + let original: Vec = serde_json::from_value(serde_json::json!([ + {"role":"user","content":[{"type":"text","text":"Analyze the data"}]}, + {"role":"assistant","content":[{"type":"tool_use","id":"call_1","name":"bash","input":{"command":"echo ready"}}]}, + {"role":"user","content":[{"type":"tool_result","tool_use_id":"call_1","content":"ready"}]}, + {"role":"assistant","content":[{"type":"tool_use","id":"call_2","name":"bash","input":{"command":"echo done"}}]}, + {"role":"user","content":[{"type":"tool_result","tool_use_id":"call_2","content":"done"}]} + ])).unwrap(); + for snapshots in [ + vec![], + vec![topology_snapshot( + "agent_alpha", + "Tide", + SubAgentStatus::Running, + )], + ] { + let mut messages = original.clone(); + replace_agent_topology_checkpoint(&mut messages, &snapshots); + assert_eq!(messages.len(), original.len() + 1); + assert_eq!(messages[0], original[0]); + assert!(is_agent_topology_checkpoint(&messages[1])); + assert_eq!( + &messages[2..], + &original[1..], + "assistant/tool chain must remain intact and end the request" + ); + let once = messages.clone(); + replace_agent_topology_checkpoint(&mut messages, &snapshots); + assert_eq!(messages, once); + } + let mut without_user = original.clone(); + without_user[0].role = Role::System; + replace_agent_topology_checkpoint(&mut without_user, &[]); + assert_eq!(without_user[0].role, Role::System); + assert!(is_agent_topology_checkpoint(&without_user[1])); + assert_eq!(&without_user[2..], &original[1..]); + } + #[test] fn empty_current_topology_explicitly_overrides_old_agent_claims() { let lookalike = Message { From 858393709c49dfb618568d915a95a0acfc02a892 Mon Sep 17 00:00:00 2001 From: z27014 Date: Wed, 16 Sep 2026 15:42:23 +0800 Subject: [PATCH 2/7] fix: normalize compacted tool rounds on chat wire Signed-off-by: z27014 --- crates/tui/src/client/chat.rs | 127 +++++++++++++++++++++++++++++- crates/tui/src/compaction.rs | 18 ++++- crates/tui/src/runtime_handoff.rs | 13 ++- 3 files changed, 151 insertions(+), 7 deletions(-) diff --git a/crates/tui/src/client/chat.rs b/crates/tui/src/client/chat.rs index 629096d1a8..0a71d2c3d4 100644 --- a/crates/tui/src/client/chat.rs +++ b/crates/tui/src/client/chat.rs @@ -2562,6 +2562,23 @@ fn last_chars(value: &str, count: usize) -> String { chars.into_iter().collect() } +fn merge_adjacent_user_content(previous: Value, current: Value) -> Value { + match (previous, current) { + (Value::String(left), Value::String(right)) => json!(format!("{left}\n\n{right}")), + (left, right) => { + let mut parts = Vec::new(); + for content in [left, right] { + match content { + Value::Array(items) => parts.extend(items), + Value::String(text) => parts.push(json!({"type": "text", "text": text})), + other => parts.push(other), + } + } + Value::Array(parts) + } + } +} + fn build_chat_messages_with_reasoning( system: Option<&SystemPrompt>, messages: &[Message], @@ -2587,7 +2604,39 @@ fn build_chat_messages_with_reasoning( })); } - for (message_index, message) in messages.iter().enumerate() { + // Compaction persists its summary after the retained round. On strict + // alternating templates that puts a user message after a tool result + // (which the template treats as the user's half of the tool exchange). + // Move only the summary on the wire, ahead of the retained user prompt; + // the saved history and matching assistant/tool call IDs stay untouched. + let summary_index = messages + .iter() + .rposition(crate::compaction::is_wire_compaction_checkpoint_message); + let summary_target = summary_index.and_then(|summary_index| { + messages[..summary_index] + .iter() + .rposition(crate::runtime_handoff::is_user_turn_prompt) + .or_else(|| { + messages[..summary_index] + .iter() + .position(|message| message.role.is_assistant_like()) + }) + }); + let wire_messages = (0..messages.len()) + .filter(|index| Some(*index) != summary_index || summary_target.is_none()) + .flat_map(|index| { + if Some(index) == summary_target { + [summary_index, Some(index)] + .into_iter() + .flatten() + .collect::>() + } else { + vec![index] + } + }); + + for message_index in wire_messages { + let message = &messages[message_index]; // Which wire channel this message belongs in is decided by the shared // placement table, not by an `if` chain local to this adapter. let placement = role_placement(&message.role, WireDialect::ChatCompletions); @@ -2787,7 +2836,22 @@ fn build_chat_messages_with_reasoning( if include_tool_budget_metadata && let Some(turn_meta) = &turn_meta_budget { msg["_turn_meta_budget"] = turn_meta_budget_json(turn_meta); } - out.push(msg); + if summary_target.is_some() + && let Some(previous) = out.last_mut() + && previous.get("role").and_then(Value::as_str) == Some("user") + { + // A compaction summary, retained prompt, and topology + // checkpoint are one user turn to a paired template. + let previous_content = previous["content"].take(); + let current_content = msg["content"].take(); + previous["content"] = + merge_adjacent_user_content(previous_content, current_content); + if let Some(meta) = msg.get("_turn_meta_budget") { + previous["_turn_meta_budget"] = meta.clone(); + } + } else { + out.push(msg); + } } } @@ -6358,6 +6422,65 @@ mod image_block_wire_tests { const DATA_URL: &str = "data:image/png;base64,QUJD"; + #[test] + fn forkguard_compaction_tool_round_has_valid_chat_wire_roles() { + let mut messages: Vec = serde_json::from_value(serde_json::json!([ + {"role":"user","content":[{"type":"text","text":"Analyze the data"}]}, + {"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":"ready"}]}, + {"role":"assistant","content":[{"type":"tool_use","id":"call_2","name":"read","input":{"path":"b.txt"}}]}, + {"role":"user","content":[{"type":"tool_result","tool_use_id":"call_2","content":"done"}]} + ])).unwrap(); + messages.push(crate::compaction::compaction_checkpoint_message( + &crate::models::SystemPrompt::Text( + crate::compaction::build_compaction_summary_block_text("Compacted summary", ""), + ), + )); + crate::runtime_handoff::replace_agent_topology_checkpoint(&mut messages, &[]); + let stored = messages.clone(); + let wire = build_chat_messages(None, &messages, "gpt-4o"); + assert_eq!( + messages, stored, + "wire normalization must not alter saved history" + ); + let roles: Vec<&str> = wire + .iter() + .map(|message| message["role"].as_str().unwrap()) + .collect(); + assert_eq!(roles, ["user", "assistant", "tool", "assistant", "tool"]); + let prompt = wire[0]["content"].as_str().unwrap(); + assert!(prompt.contains("Compacted summary")); + assert!(prompt.contains("Analyze the data")); + assert!(prompt.contains("codewhale.agent_topology.v1")); + assert_eq!(wire[1]["tool_calls"][0]["id"], "call_1"); + assert_eq!(wire[2]["tool_call_id"], "call_1"); + assert_eq!(wire[3]["tool_calls"][0]["id"], "call_2"); + assert_eq!(wire[4]["tool_call_id"], "call_2"); + } + + #[test] + fn compaction_marker_quoted_by_user_keeps_wire_order() { + let messages: Vec = serde_json::from_value(serde_json::json!([ + {"role":"user","content":[{"type":"text","text":"First question"}]}, + {"role":"assistant","content":[{"type":"text","text":"First answer"}]}, + {"role":"user","content":[{"type":"text","text":"Please explain the phrase: Another language model started to solve this problem"}]}, + {"role":"assistant","content":[{"type":"text","text":"It introduces a summary."}]}, + {"role":"user","content":[{"type":"text","text":"Follow-up question"}]} + ])).unwrap(); + let wire = build_chat_messages(None, &messages, "gpt-4o"); + let roles: Vec<&str> = wire + .iter() + .map(|message| message["role"].as_str().unwrap()) + .collect(); + assert_eq!(roles, ["user", "assistant", "user", "assistant", "user"]); + assert_eq!(wire[0]["content"], "First question"); + assert_eq!( + wire[2]["content"], + "Please explain the phrase: Another language model started to solve this problem" + ); + assert_eq!(wire[4]["content"], "Follow-up question"); + } + fn fixture_tool_use(id: &str) -> ContentBlock { ContentBlock::ToolUse { id: id.to_string(), diff --git a/crates/tui/src/compaction.rs b/crates/tui/src/compaction.rs index 480cc79516..8198ff08f5 100644 --- a/crates/tui/src/compaction.rs +++ b/crates/tui/src/compaction.rs @@ -294,6 +294,22 @@ pub(crate) fn is_compaction_checkpoint_message(message: &Message) -> bool { user_text_of(message).is_some_and(|text| is_compaction_summary_text(&text)) } +/// Narrow recognition for request-time reordering. The broader legacy marker +/// scan above is needed while replacing old summaries, but must not turn an +/// ordinary user quoting that marker into a movable wire checkpoint. +pub(crate) fn is_wire_compaction_checkpoint_message(message: &Message) -> bool { + let [ + ContentBlock::Text { + text, + cache_control: None, + }, + ] = message.content.as_slice() + else { + return false; + }; + message.role == Role::User && text.starts_with(SUMMARY_HEADER) +} + pub(crate) fn estimate_tokens_for_message(message: &Message, include_thinking: bool) -> usize { message .content @@ -1151,7 +1167,7 @@ pub async fn compact_messages_safe( .unwrap_or_else(|| anyhow::anyhow!("Compaction failed after {MAX_RETRIES} retries"))) } -fn build_compaction_summary_block_text(summary: &str, anchors: &str) -> String { +pub(crate) fn build_compaction_summary_block_text(summary: &str, anchors: &str) -> String { let summary = summary.trim(); let summary = if summary.is_empty() { "(no summary available)" diff --git a/crates/tui/src/runtime_handoff.rs b/crates/tui/src/runtime_handoff.rs index daa0919ced..0b22fdc801 100644 --- a/crates/tui/src/runtime_handoff.rs +++ b/crates/tui/src/runtime_handoff.rs @@ -456,8 +456,9 @@ fn is_agent_topology_checkpoint(message: &Message) -> bool { /// compaction. A current empty topology is still meaningful: it overrides a /// narrative summary or old runtime event that says an Agent remains live. /// Replays are idempotent because the previous sidecar is structurally removed -/// before the replacement is inserted. At a tool-result boundary, place it -/// after the latest user input, before that round's assistant/tool chain. +/// before the replacement is inserted. At a tool-result or trailing summary +/// boundary, place it after the latest real user input, before that round's +/// assistant/tool chain. /// Strict paired chat templates cannot encode a tool result followed by a /// user checkpoint. Compaction already replaces history; this does not mutate /// the session-pinned system/cache prefix or impersonate an assistant reply. @@ -476,11 +477,15 @@ pub(crate) fn replace_agent_topology_checkpoint( ) }) }); - let position = if ends_with_tool_result { + let ends_with_compaction_summary = messages + .last() + .is_some_and(crate::compaction::is_wire_compaction_checkpoint_message); + let position = if ends_with_tool_result || ends_with_compaction_summary { messages .iter() .rposition(|message| { - classify_user_turn_prompt(message) != UserTurnPromptKind::NotPrompt + !crate::compaction::is_wire_compaction_checkpoint_message(message) + && classify_user_turn_prompt(message) != UserTurnPromptKind::NotPrompt }) .map_or_else( || { From c2dafb6e0b4b05c0f0868c63a44a50429164bc8e Mon Sep 17 00:00:00 2001 From: z27014 Date: Wed, 16 Sep 2026 17:44:25 +0800 Subject: [PATCH 3/7] fix: preserve real user boundary during recompaction Signed-off-by: z27014 --- crates/tui/src/client/chat.rs | 120 ++++++++++++++++++++++-- crates/tui/src/compaction.rs | 3 + crates/tui/src/compaction/last_round.rs | 66 +++++++++++-- crates/tui/src/runtime_handoff.rs | 9 +- 4 files changed, 180 insertions(+), 18 deletions(-) diff --git a/crates/tui/src/client/chat.rs b/crates/tui/src/client/chat.rs index 0a71d2c3d4..73f1e46db8 100644 --- a/crates/tui/src/client/chat.rs +++ b/crates/tui/src/client/chat.rs @@ -2607,15 +2607,25 @@ fn build_chat_messages_with_reasoning( // Compaction persists its summary after the retained round. On strict // alternating templates that puts a user message after a tool result // (which the template treats as the user's half of the tool exchange). - // Move only the summary on the wire, ahead of the retained user prompt; - // the saved history and matching assistant/tool call IDs stay untouched. - let summary_index = messages - .iter() - .rposition(crate::compaction::is_wire_compaction_checkpoint_message); + // Move only a trailing summary after a tool result on the wire, ahead of + // the retained user prompt. A user pasting a summary header in a normal + // chat turn must keep its original position. Saved history and matching + // assistant/tool call IDs stay untouched. + let summary_index = messages.len().checked_sub(1).filter(|&index| { + crate::compaction::is_wire_compaction_checkpoint_message(&messages[index]) + && index > 0 + && messages[index - 1] + .content + .iter() + .any(|block| matches!(block, ContentBlock::ToolResult { .. })) + }); let summary_target = summary_index.and_then(|summary_index| { messages[..summary_index] .iter() - .rposition(crate::runtime_handoff::is_user_turn_prompt) + .rposition(|message| { + crate::runtime_handoff::classify_user_turn_prompt(message) + != crate::runtime_handoff::UserTurnPromptKind::NotPrompt + }) .or_else(|| { messages[..summary_index] .iter() @@ -2836,17 +2846,21 @@ fn build_chat_messages_with_reasoning( if include_tool_budget_metadata && let Some(turn_meta) = &turn_meta_budget { msg["_turn_meta_budget"] = turn_meta_budget_json(turn_meta); } - if summary_target.is_some() + if (Some(message_index) == summary_target + || crate::runtime_handoff::is_agent_topology_checkpoint(message)) && let Some(previous) = out.last_mut() && previous.get("role").and_then(Value::as_str) == Some("user") { - // A compaction summary, retained prompt, and topology - // checkpoint are one user turn to a paired template. + // Merge only this retained prompt with its moved summary, + // or an adjacent topology checkpoint (also in prune-only + // compaction). Other user turns keep their boundaries. let previous_content = previous["content"].take(); let current_content = msg["content"].take(); previous["content"] = merge_adjacent_user_content(previous_content, current_content); - if let Some(meta) = msg.get("_turn_meta_budget") { + if previous.get("_turn_meta_budget").is_none() + && let Some(meta) = msg.get("_turn_meta_budget") + { previous["_turn_meta_budget"] = meta.clone(); } } else { @@ -6481,6 +6495,92 @@ mod image_block_wire_tests { assert_eq!(wire[4]["content"], "Follow-up question"); } + #[test] + fn exact_summary_header_pasted_by_user_keeps_wire_order() { + let pasted = crate::compaction::build_compaction_summary_block_text( + "Please explain this old summary", + "", + ); + let messages = vec![ + Message { + role: crate::models::Role::User, + content: vec![crate::models::ContentBlock::Text { + text: "Earlier question".into(), + cache_control: None, + }], + }, + Message { + role: crate::models::Role::Assistant, + content: vec![crate::models::ContentBlock::Text { + text: "Earlier answer".into(), + cache_control: None, + }], + }, + Message { + role: crate::models::Role::User, + content: vec![crate::models::ContentBlock::Text { + text: pasted.clone(), + cache_control: None, + }], + }, + ]; + let wire = build_chat_messages(None, &messages, "gpt-4o"); + assert_eq!(wire.len(), 3); + assert_eq!(wire[0]["content"], "Earlier question"); + assert_eq!(wire[1]["role"], "assistant"); + assert_eq!(wire[2]["content"], pasted); + } + + #[test] + fn prune_only_topology_merges_with_its_prompt_on_wire() { + let mut messages: Vec = serde_json::from_value(serde_json::json!([ + {"role":"user","content":[{"type":"text","text":"Run the suite"}]}, + {"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(); + crate::runtime_handoff::replace_agent_topology_checkpoint(&mut messages, &[]); + let wire = build_chat_messages(None, &messages, "gpt-4o"); + let roles: Vec<_> = wire + .iter() + .map(|message| message["role"].as_str().unwrap()) + .collect(); + assert_eq!(roles, ["user", "assistant", "tool"]); + assert!( + wire[0]["content"] + .as_str() + .unwrap() + .contains("codewhale.agent_topology.v1") + ); + assert_eq!(wire[1]["tool_calls"][0]["id"], wire[2]["tool_call_id"]); + } + + #[test] + fn compaction_does_not_merge_unrelated_adjacent_user_messages() { + let mut messages: Vec = serde_json::from_value(serde_json::json!([ + {"role":"user","content":[{"type":"text","text":"Earlier user text"}]}, + {"role":"user","content":[{"type":"text","text":"Run the suite"}]}, + {"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("Summary", ""), + ), + )); + let wire = build_chat_messages(None, &messages, "gpt-4o"); + assert_eq!(wire[0]["role"], "user"); + assert_eq!(wire[0]["content"], "Earlier user text"); + assert_eq!(wire[1]["role"], "user"); + assert!( + wire[1]["content"] + .as_str() + .unwrap() + .contains("Run the suite") + ); + assert_eq!(wire[2]["role"], "assistant"); + assert_eq!(wire[3]["role"], "tool"); + } + fn fixture_tool_use(id: &str) -> ContentBlock { ContentBlock::ToolUse { id: id.to_string(), diff --git a/crates/tui/src/compaction.rs b/crates/tui/src/compaction.rs index 8198ff08f5..97abd54217 100644 --- a/crates/tui/src/compaction.rs +++ b/crates/tui/src/compaction.rs @@ -1190,6 +1190,9 @@ pub(crate) fn retained_user_messages(messages: &[Message], max_tokens: usize) -> if remaining == 0 { break; } + if !crate::runtime_handoff::is_user_turn_prompt(msg) { + continue; + } let Some(text) = user_text_of(msg) else { continue; }; diff --git a/crates/tui/src/compaction/last_round.rs b/crates/tui/src/compaction/last_round.rs index adbd8635f6..f44c114588 100644 --- a/crates/tui/src/compaction/last_round.rs +++ b/crates/tui/src/compaction/last_round.rs @@ -78,7 +78,10 @@ pub fn inspect_compaction_keep(messages: &[Message]) -> CompactionKeep { let last_round = last_round_slice(messages); CompactionKeep { has_checkpoint: messages.iter().any(is_compaction_checkpoint_message), - last_round_messages: last_round.len(), + last_round_messages: last_round + .iter() + .filter(|message| is_round_message(message)) + .count(), last_round_tool_results: last_round.iter().flat_map(tool_result_ids).count(), last_round_assistant: last_round .iter() @@ -102,7 +105,20 @@ pub fn pinned_anchors_text(workspace: Option<&std::path::Path>) -> Option bool { - !is_compaction_checkpoint_message(message) && user_text_of(message).is_some() + !is_compaction_checkpoint_message(message) + && crate::runtime_handoff::is_user_turn_prompt(message) + && user_text_of(message).is_some() +} + +fn user_prompt_text_of(message: &Message) -> Option { + is_plain_user_text(message) + .then(|| user_text_of(message)) + .flatten() +} + +fn is_round_message(message: &Message) -> bool { + !crate::runtime_handoff::is_internal_runtime_handoff(message) + && !is_compaction_checkpoint_message(message) } fn last_plain_user_index(messages: &[Message], end: usize) -> Option { @@ -166,7 +182,12 @@ pub fn last_round_kept_count(messages: &[Message]) -> Option { return None; } let start = last_round_start(&messages[..checkpoint]); - Some(checkpoint.saturating_sub(start)) + Some( + messages[start..checkpoint] + .iter() + .filter(|message| is_round_message(message)) + .count(), + ) } fn last_round_slice(messages: &[Message]) -> &[Message] { @@ -283,8 +304,8 @@ pub(crate) fn validate_last_round_coverage( // tool-bearing turn, so the round routinely spans two user messages -- and // checking only the earliest let a rewrite drop the *latest* one, which is // the turn this whole contract exists to keep. - for text in last_round.iter().filter_map(user_text_of) { - if !survives(&text, replacement, user_text_of) { + for text in last_round.iter().filter_map(user_prompt_text_of) { + if !survives(&text, replacement, user_prompt_text_of) { anyhow::bail!( "Compaction coverage floor: a last-round user message was dropped; history was not replaced." ); @@ -393,7 +414,10 @@ pub(super) fn measure_coverage( let last_round = last_round_slice(replacement); CompactionCoverage { path, - last_round_messages: last_round.len(), + last_round_messages: last_round + .iter() + .filter(|message| is_round_message(message)) + .count(), last_round_tool_results: last_round.iter().flat_map(tool_result_ids).count(), last_round_assistant: last_round .iter() @@ -606,6 +630,36 @@ mod tests { })); } + #[test] + fn forkguard_mid_round_topology_is_not_a_user_turn_on_recompaction() { + let mut messages = vec![ + msg("user", "Run the suite now."), + tool_use("live", "Bash", json!({"command": "cargo test"})), + tool_result("live", "ok"), + compaction_checkpoint_message(&SystemPrompt::Text( + crate::compaction::build_compaction_summary_block_text("suite finished", ""), + )), + ]; + crate::runtime_handoff::replace_agent_topology_checkpoint(&mut messages, &[]); + assert!(crate::runtime_handoff::is_internal_runtime_handoff( + &messages[1] + )); + assert_eq!(last_round_start(&messages), 0); + assert_eq!(inspect_compaction_keep(&messages).last_round_messages, 3); + assert_eq!( + measure_coverage(&messages, &messages, CompactionPath::Summary, 0).last_round_messages, + 3 + ); + + let without_prompt = messages[1..].to_vec(); + assert!(validate_last_round_coverage(&messages, &without_prompt).is_err()); + + messages.push(msg("user", "One more question.")); + let retained = retained_user_messages(&messages, 10_000); + let texts: Vec<_> = retained.iter().filter_map(user_text_of).collect(); + assert_eq!(texts, ["Run the suite now.", "One more question."]); + } + #[test] fn last_round_walks_back_through_toolless_tails_to_the_tool_round() { let original = vec![ diff --git a/crates/tui/src/runtime_handoff.rs b/crates/tui/src/runtime_handoff.rs index 0b22fdc801..652cc89e79 100644 --- a/crates/tui/src/runtime_handoff.rs +++ b/crates/tui/src/runtime_handoff.rs @@ -430,7 +430,7 @@ fn render_restored_agent_topology(checkpoint: &SavedAgentTopologyCheckpoint) -> display } -fn is_agent_topology_checkpoint(message: &Message) -> bool { +pub(crate) fn is_agent_topology_checkpoint(message: &Message) -> bool { let [ ContentBlock::Text { text, @@ -492,7 +492,7 @@ pub(crate) fn replace_agent_topology_checkpoint( messages .iter() .position(|message| message.role.is_assistant_like()) - .unwrap_or(messages.len()) + .unwrap_or(0) }, |index| index + 1, ) @@ -1311,6 +1311,11 @@ mod tests { assert_eq!(without_user[0].role, Role::System); assert!(is_agent_topology_checkpoint(&without_user[1])); assert_eq!(&without_user[2..], &original[1..]); + + let mut corrupt_tool_only = vec![original[2].clone()]; + replace_agent_topology_checkpoint(&mut corrupt_tool_only, &[]); + assert!(is_agent_topology_checkpoint(&corrupt_tool_only[0])); + assert_eq!(corrupt_tool_only[1], original[2]); } #[test] From 68cc419d445f2c8562e61929bf8b12f890da3031 Mon Sep 17 00:00:00 2001 From: z27014 Date: Wed, 16 Sep 2026 18:44:55 +0800 Subject: [PATCH 4/7] fix: preserve compacted turns across restores Signed-off-by: z27014 --- crates/tui/src/client/chat.rs | 100 ++++++++++++++++++++---- crates/tui/src/compaction.rs | 17 ++++ crates/tui/src/compaction/last_round.rs | 74 +++++++++++++----- crates/tui/src/core/engine.rs | 17 ++-- crates/tui/src/core/engine/tests.rs | 18 ++++- 5 files changed, 183 insertions(+), 43 deletions(-) diff --git a/crates/tui/src/client/chat.rs b/crates/tui/src/client/chat.rs index 73f1e46db8..d6b21b8df5 100644 --- a/crates/tui/src/client/chat.rs +++ b/crates/tui/src/client/chat.rs @@ -2607,18 +2607,25 @@ fn build_chat_messages_with_reasoning( // Compaction persists its summary after the retained round. On strict // alternating templates that puts a user message after a tool result // (which the template treats as the user's half of the tool exchange). - // Move only a trailing summary after a tool result on the wire, ahead of - // the retained user prompt. A user pasting a summary header in a normal - // chat turn must keep its original position. Saved history and matching + // A persisted summary may no longer be the final message once another + // turn is appended. Move the latest summary that directly follows a tool + // result ahead of its retained prompt on every request, not just the first + // request after compaction. Ordinary user text containing the summary + // header in a chat turn keeps its original position. Saved history and // assistant/tool call IDs stay untouched. - let summary_index = messages.len().checked_sub(1).filter(|&index| { - crate::compaction::is_wire_compaction_checkpoint_message(&messages[index]) - && index > 0 - && messages[index - 1] - .content - .iter() - .any(|block| matches!(block, ContentBlock::ToolResult { .. })) - }); + let summary_index = messages + .iter() + .enumerate() + .rev() + .find_map(|(index, message)| { + (index > 0 + && crate::compaction::is_wire_compaction_checkpoint_message(message) + && messages[index - 1] + .content + .iter() + .any(|block| matches!(block, ContentBlock::ToolResult { .. }))) + .then_some(index) + }); let summary_target = summary_index.and_then(|summary_index| { messages[..summary_index] .iter() @@ -2847,13 +2854,15 @@ fn build_chat_messages_with_reasoning( msg["_turn_meta_budget"] = turn_meta_budget_json(turn_meta); } if (Some(message_index) == summary_target - || crate::runtime_handoff::is_agent_topology_checkpoint(message)) + || crate::runtime_handoff::is_agent_topology_checkpoint(message) + || crate::runtime_handoff::restored_subagent_checkpoint_display(message) + .is_some()) && let Some(previous) = out.last_mut() && previous.get("role").and_then(Value::as_str) == Some("user") { // Merge only this retained prompt with its moved summary, - // or an adjacent topology checkpoint (also in prune-only - // compaction). Other user turns keep their boundaries. + // or an adjacent runtime checkpoint (including restored + // topology). Other user turns keep their boundaries. let previous_content = previous["content"].take(); let current_content = msg["content"].take(); previous["content"] = @@ -6470,6 +6479,69 @@ mod image_block_wire_tests { assert_eq!(wire[2]["tool_call_id"], "call_1"); assert_eq!(wire[3]["tool_calls"][0]["id"], "call_2"); assert_eq!(wire[4]["tool_call_id"], "call_2"); + + messages.push( + serde_json::from_value(serde_json::json!({ + "role":"assistant","content":[{"type":"text","text":"Analysis complete"}] + })) + .unwrap(), + ); + messages.push( + serde_json::from_value(serde_json::json!({ + "role":"user","content":[{"type":"text","text":"What happened next?"}] + })) + .unwrap(), + ); + let later_wire = build_chat_messages(None, &messages, "gpt-4o"); + let later_roles: Vec<&str> = later_wire + .iter() + .map(|message| message["role"].as_str().unwrap()) + .collect(); + assert_eq!( + later_roles, + [ + "user", + "assistant", + "tool", + "assistant", + "tool", + "assistant", + "user" + ] + ); + assert!( + later_wire[0]["content"] + .as_str() + .unwrap() + .contains("Compacted summary") + ); + assert_eq!(later_wire[6]["content"], "What happened next?"); + + let restored = crate::compaction::restore_compaction_checkpoint( + crate::runtime_handoff::project_messages_for_restore(&messages), + Some(&crate::models::SystemPrompt::Text( + crate::compaction::build_compaction_summary_block_text("Compacted summary", ""), + )), + ); + let restored_wire = build_chat_messages(None, &restored, "gpt-4o"); + let restored_roles: Vec<&str> = restored_wire + .iter() + .map(|message| message["role"].as_str().unwrap()) + .collect(); + assert_eq!(restored_roles, later_roles); + assert!( + restored_wire[0]["content"] + .as_str() + .unwrap() + .contains("Compacted summary") + ); + assert!( + restored_wire[0]["content"] + .as_str() + .unwrap() + .contains("restored Agent topology checkpoint") + ); + assert_eq!(restored_wire[6]["content"], "What happened next?"); } #[test] diff --git a/crates/tui/src/compaction.rs b/crates/tui/src/compaction.rs index 97abd54217..8b8e909f75 100644 --- a/crates/tui/src/compaction.rs +++ b/crates/tui/src/compaction.rs @@ -310,6 +310,23 @@ pub(crate) fn is_wire_compaction_checkpoint_message(message: &Message) -> bool { message.role == Role::User && text.starts_with(SUMMARY_HEADER) } +/// Replace the saved history checkpoint with the authoritative carrier while +/// keeping its position relative to later turns after a session restore. +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)); + if let Some(checkpoint) = checkpoint { + let index = checkpoint_index + .unwrap_or(messages.len()) + .min(messages.len()); + messages.insert(index, compaction_checkpoint_message(checkpoint)); + } + messages +} + pub(crate) fn estimate_tokens_for_message(message: &Message, include_thinking: bool) -> usize { message .content diff --git a/crates/tui/src/compaction/last_round.rs b/crates/tui/src/compaction/last_round.rs index f44c114588..2b0119a908 100644 --- a/crates/tui/src/compaction/last_round.rs +++ b/crates/tui/src/compaction/last_round.rs @@ -165,29 +165,24 @@ pub(crate) fn last_round_start(messages: &[Message]) -> usize { #[must_use] pub(crate) fn last_round_range(messages: &[Message]) -> (usize, usize) { let start = last_round_start(messages).min(messages.len()); - let end = messages[start..] - .iter() - .position(is_compaction_checkpoint_message) - .map_or(messages.len(), |rel| start + rel); - (start, end) + // An older summary can sit in the middle once later turns are appended. + // The whole suffix must remain in the coverage floor; the old summary is + // removed when building the replacement, not used as a cut point. + (start, messages.len()) } /// How many messages of the open round sit in `messages` before a checkpoint. #[must_use] pub fn last_round_kept_count(messages: &[Message]) -> Option { - let checkpoint = messages + messages .iter() - .rposition(is_compaction_checkpoint_message)?; - if checkpoint == 0 { - return None; - } - let start = last_round_start(&messages[..checkpoint]); - Some( - messages[start..checkpoint] - .iter() - .filter(|message| is_round_message(message)) - .count(), - ) + .any(is_compaction_checkpoint_message) + .then(|| { + last_round_slice(messages) + .iter() + .filter(|message| is_round_message(message)) + .count() + }) } fn last_round_slice(messages: &[Message]) -> &[Message] { @@ -196,7 +191,11 @@ fn last_round_slice(messages: &[Message]) -> &[Message] { } pub(super) fn bound_last_round(messages: &[Message]) -> Vec { - let mut round = messages.to_vec(); + let mut round: Vec<_> = messages + .iter() + .filter(|message| !is_compaction_checkpoint_message(message)) + .cloned() + .collect(); for message in &mut round { for block in &mut message.content { if let ContentBlock::ToolResult { @@ -660,6 +659,45 @@ mod tests { assert_eq!(texts, ["Run the suite now.", "One more question."]); } + #[test] + fn forkguard_recompaction_keeps_turns_after_the_previous_summary() { + let mut messages = vec![ + msg("user", "Run the suite now."), + tool_use("live", "Bash", json!({"command": "cargo test"})), + tool_result("live", "ok"), + compaction_checkpoint_message(&SystemPrompt::Text( + crate::compaction::build_compaction_summary_block_text("suite finished", ""), + )), + ]; + crate::runtime_handoff::replace_agent_topology_checkpoint(&mut messages, &[]); + messages.push(msg("assistant", "The suite passed.")); + messages.push(msg("user", "Explain the next step.")); + messages.push(msg("assistant", "Check the release notes.")); + + let (start, end) = last_round_range(&messages); + assert_eq!((start, end), (0, messages.len())); + let next = crate::compaction::build_compaction_summary_block_text("new summary", ""); + let replacement = build_replacement_history(&messages, &next, None).unwrap(); + let texts: Vec<_> = replacement.iter().filter_map(user_text_of).collect(); + assert!(texts.iter().any(|text| text == "Run the suite now.")); + assert!(texts.iter().any(|text| text == "Explain the next step.")); + assert!(replacement.iter().any(|message| { + assistant_text_of(message).as_deref() == Some("Check the release notes.") + })); + assert_eq!( + replacement + .iter() + .filter(|message| is_compaction_checkpoint_message(message)) + .count(), + 1 + ); + let dropped_new_turn: Vec<_> = replacement + .into_iter() + .filter(|message| user_text_of(message).as_deref() != Some("Explain the next step.")) + .collect(); + assert!(validate_last_round_coverage(&messages, &dropped_new_turn).is_err()); + } + #[test] fn last_round_walks_back_through_toolless_tails_to_the_tool_round() { let original = vec![ diff --git a/crates/tui/src/core/engine.rs b/crates/tui/src/core/engine.rs index 2b5deb2389..442b23c6a9 100644 --- a/crates/tui/src/core/engine.rs +++ b/crates/tui/src/core/engine.rs @@ -3427,18 +3427,15 @@ impl Engine { } let compaction_checkpoint = extract_compaction_summary_prompt(system_prompt.clone()); - let mut restored_messages = + let restored_messages = crate::runtime_handoff::project_messages_for_restore(&messages); // The persisted carrier is authoritative for the one - // history checkpoint. Drop stale projected copies so - // repeated reloads cannot stack or retain an older one. - restored_messages.retain(|message| { - !crate::compaction::is_compaction_checkpoint_message(message) - }); - if let Some(checkpoint) = compaction_checkpoint.as_ref() { - restored_messages - .push(crate::compaction::compaction_checkpoint_message(checkpoint)); - } + // history checkpoint, at its saved location. Later + // turns must remain after it for Chat wire ordering. + let restored_messages = crate::compaction::restore_compaction_checkpoint( + restored_messages, + compaction_checkpoint.as_ref(), + ); self.session.messages = restored_messages.into(); // Direct field assignment bypasses `add_message` / // `replace_messages`, which own the messages-revision diff --git a/crates/tui/src/core/engine/tests.rs b/crates/tui/src/core/engine/tests.rs index 91cc6c7354..a9541a0f33 100644 --- a/crates/tui/src/core/engine/tests.rs +++ b/crates/tui/src/core/engine/tests.rs @@ -15654,7 +15654,17 @@ async fn sync_session_migrates_one_checkpoint_and_strips_its_system_carrier() { )); let run = tokio::spawn(engine.run()); - let mut messages = vec![old_checkpoint]; + let mut messages = vec![ + old_checkpoint, + serde_json::from_value(json!({ + "role": "assistant", "content": [{"type": "text", "text": "Later answer"}] + })) + .unwrap(), + serde_json::from_value(json!({ + "role": "user", "content": [{"type": "text", "text": "Later question"}] + })) + .unwrap(), + ]; for round in 0..2 { handle .send(Op::SyncSession { @@ -15687,6 +15697,12 @@ async fn sync_session_migrates_one_checkpoint_and_strips_its_system_carrier() { .filter(|message| crate::compaction::is_compaction_checkpoint_message(message)) .collect::>(); assert_eq!(checkpoints.len(), 1, "round {round}: {checkpoints:?}"); + assert!( + crate::compaction::is_compaction_checkpoint_message(&snapshot.messages[0]), + "round {round}: later turns must remain after the saved checkpoint" + ); + assert_eq!(message_text_of(&snapshot.messages[1]), "Later answer"); + assert_eq!(message_text_of(&snapshot.messages[2]), "Later question"); let checkpoint_text = message_text_of(checkpoints[0]); assert!( checkpoint_text.contains("new checkpoint"), From 6dd37964b7b8eff31ca45a4651a7d6dcaa5f54f7 Mon Sep 17 00:00:00 2001 From: z27014 Date: Wed, 16 Sep 2026 19:07:30 +0800 Subject: [PATCH 5/7] fix: identify generated compaction summaries Signed-off-by: z27014 --- crates/tui/src/client/chat.rs | 33 +++++++++++++++++++++++++++++++-- crates/tui/src/compaction.rs | 34 +++++++++++++++++++++++++--------- 2 files changed, 56 insertions(+), 11 deletions(-) diff --git a/crates/tui/src/client/chat.rs b/crates/tui/src/client/chat.rs index d6b21b8df5..bb593c8af3 100644 --- a/crates/tui/src/client/chat.rs +++ b/crates/tui/src/client/chat.rs @@ -2611,8 +2611,9 @@ fn build_chat_messages_with_reasoning( // turn is appended. Move the latest summary that directly follows a tool // result ahead of its retained prompt on every request, not just the first // request after compaction. Ordinary user text containing the summary - // header in a chat turn keeps its original position. Saved history and - // assistant/tool call IDs stay untouched. + // header in a chat turn keeps its original position, even when it follows + // a tool result: only generated checkpoints carry the provenance block. + // Saved history and assistant/tool call IDs stay untouched. let summary_index = messages .iter() .enumerate() @@ -6603,6 +6604,34 @@ mod image_block_wire_tests { assert_eq!(wire[2]["content"], pasted); } + #[test] + fn forkguard_full_summary_header_pasted_after_tool_result_is_not_relocated() { + 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, + }], + }); + let wire = build_chat_messages(None, &messages, "gpt-4o"); + let roles: Vec<_> = wire + .iter() + .map(|message| message["role"].as_str().unwrap()) + .collect(); + assert_eq!(roles, ["user", "assistant", "tool", "user"]); + assert_eq!(wire[0]["content"], "Run the tool"); + assert_eq!(wire[3]["content"], pasted); + assert_eq!(wire[2]["tool_call_id"], "call_1"); + } + #[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 8b8e909f75..bedadb5313 100644 --- a/crates/tui/src/compaction.rs +++ b/crates/tui/src/compaction.rs @@ -167,6 +167,7 @@ pub const COMPACTION_SUMMARY_MARKER: &str = "Another language model started to s /// Marker written by pre-v0.9.6 compaction; sessions saved under the old /// format must still be recognized so their summary is replaced, not stacked. pub const LEGACY_COMPACTION_SUMMARY_MARKER: &str = "Conversation Summary (Auto-Generated)"; +const COMPACTION_CHECKPOINT_PROVENANCE: &str = ""; const COMPACTION_SUMMARY_BEGIN: &str = ""; const COMPACTION_SUMMARY_END: &str = ""; @@ -282,10 +283,16 @@ pub fn summary_prompt_text(prompt: &SystemPrompt) -> String { pub(crate) fn compaction_checkpoint_message(prompt: &SystemPrompt) -> Message { Message { role: Role::User, - content: vec![ContentBlock::Text { - text: summary_prompt_text(prompt), - cache_control: None, - }], + content: vec![ + ContentBlock::Text { + text: summary_prompt_text(prompt), + cache_control: None, + }, + ContentBlock::Text { + text: COMPACTION_CHECKPOINT_PROVENANCE.to_string(), + cache_control: None, + }, + ], } } @@ -303,11 +310,17 @@ pub(crate) fn is_wire_compaction_checkpoint_message(message: &Message) -> bool { text, cache_control: None, }, + ContentBlock::Text { + text: provenance, + cache_control: None, + }, ] = message.content.as_slice() else { return false; }; - message.role == Role::User && text.starts_with(SUMMARY_HEADER) + message.role == Role::User + && text.starts_with(SUMMARY_HEADER) + && provenance == COMPACTION_CHECKPOINT_PROVENANCE } /// Replace the saved history checkpoint with the authoritative carrier while @@ -2069,10 +2082,13 @@ mod tests { }) })); assert!(is_compaction_checkpoint_message(retained.last().unwrap())); - assert_eq!( - user_text_of(retained.last().unwrap()).as_deref(), - Some(text.as_str()) - ); + assert!(is_wire_compaction_checkpoint_message( + retained.last().unwrap() + )); + let ContentBlock::Text { text: body, .. } = &retained.last().unwrap().content[0] else { + panic!("compaction checkpoint must begin with its summary"); + }; + assert_eq!(body, text); last_round::validate_last_round_coverage(&messages, &retained[..retained.len() - 1]) .unwrap(); } From e549f3477bb4442c67bfa2c17f70ebb7ee16b9ed Mon Sep 17 00:00:00 2001 From: z27014 Date: Wed, 16 Sep 2026 19:38:28 +0800 Subject: [PATCH 6/7] fix: scope restored topology wire merging Signed-off-by: z27014 --- crates/tui/src/client/chat.rs | 34 +++++++++++++++++++++++++++---- crates/tui/src/runtime_handoff.rs | 7 +++++++ 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/crates/tui/src/client/chat.rs b/crates/tui/src/client/chat.rs index bb593c8af3..2979aa47ff 100644 --- a/crates/tui/src/client/chat.rs +++ b/crates/tui/src/client/chat.rs @@ -2856,14 +2856,13 @@ fn build_chat_messages_with_reasoning( } if (Some(message_index) == summary_target || crate::runtime_handoff::is_agent_topology_checkpoint(message) - || crate::runtime_handoff::restored_subagent_checkpoint_display(message) - .is_some()) + || crate::runtime_handoff::is_restored_agent_topology_checkpoint(message)) && let Some(previous) = out.last_mut() && previous.get("role").and_then(Value::as_str) == Some("user") { // Merge only this retained prompt with its moved summary, - // or an adjacent runtime checkpoint (including restored - // topology). Other user turns keep their boundaries. + // or an adjacent topology checkpoint, including its + // restored form. Other user turns keep their boundaries. let previous_content = previous["content"].take(); let current_content = msg["content"].take(); previous["content"] = @@ -6545,6 +6544,33 @@ mod image_block_wire_tests { assert_eq!(restored_wire[6]["content"], "What happened next?"); } + #[test] + fn restored_completion_does_not_join_an_ordinary_user_turn() { + let prompt: Message = serde_json::from_value(serde_json::json!({ + "role":"user","content":[{"type":"text","text":"Continue the task"}] + })) + .unwrap(); + let completion = crate::runtime_handoff::subagent_completion_runtime_message( + "unavailable historical completion", + ); + let restored = crate::runtime_handoff::project_messages_for_restore(&[prompt, completion]); + assert!( + crate::runtime_handoff::restored_subagent_checkpoint_display(&restored[1]).is_some() + ); + assert!(!crate::runtime_handoff::is_restored_agent_topology_checkpoint(&restored[1])); + + let wire = build_chat_messages(None, &restored, "gpt-4o"); + assert_eq!(wire.len(), 2); + assert_eq!(wire[0]["content"], "Continue the task"); + assert_eq!(wire[1]["role"], "user"); + assert!( + wire[1]["content"] + .as_str() + .unwrap() + .contains("restored sub-agent checkpoint") + ); + } + #[test] fn compaction_marker_quoted_by_user_keeps_wire_order() { let messages: Vec = serde_json::from_value(serde_json::json!([ diff --git a/crates/tui/src/runtime_handoff.rs b/crates/tui/src/runtime_handoff.rs index 652cc89e79..70644d01b8 100644 --- a/crates/tui/src/runtime_handoff.rs +++ b/crates/tui/src/runtime_handoff.rs @@ -991,6 +991,13 @@ pub(crate) fn restored_subagent_checkpoint_display(message: &Message) -> Option< Some(text) } +/// Only the restored topology sidecar belongs to the compaction prompt +/// cluster. Other restored Agent events keep their own message boundaries. +pub(crate) fn is_restored_agent_topology_checkpoint(message: &Message) -> bool { + restored_subagent_checkpoint_display(message) + .is_some_and(|display| display.starts_with(RESTORED_TOPOLOGY_HEADER)) +} + /// Classification used when locating a user-authored turn in the session log. /// /// Runtime and tool messages are skipped because their provider-compatible From 0d679a5e835f7021d61fb82346a19d86eb784d19 Mon Sep 17 00:00:00 2001 From: z27014 Date: Thu, 17 Sep 2026 19:00:10 +0800 Subject: [PATCH 7/7] fix(tui): repair the compaction checkpoint tiers and pre-fix restore order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The checkpoint is one engine-owned history message, but the tiers that find, delete, and replace it disagreed with the tier that reorders it. Restore and recompaction decided by marker substring, so an ordinary user turn that quoted the header was deleted — silently, because the coverage floor used the same substring and exempted the message it lost — and `/edit` still treated the generated summary as an editable turn. Recognize the carrier by structure instead: a user message whose first text block begins with the summary header and whose remaining block, if any, is the engine-written provenance marker. Sessions saved before that marker existed carry the bare summary, so the single-block header-prefix form stays recognized; only the provenance-stamped form may be relocated or merged on the wire, which is what keeps a genuine turn that pastes a header in place. Sessions saved before the placement fix also stay broken across restore: their topology sidecar was appended to the end of the replacement history, request rewriting moves only the summary, and the first request after restore is `tool -> user` again. Relocate the restored carrier to the placement anchor — after the prompt its summary merges into, before that round's assistant/tool chain — and teach the recompaction replay to drop a superseded restored carrier instead of stacking a second one. One anchor definition now serves save-time placement, restore repair, and request-time relocation, replacing three copies that had already drifted on which messages count as a prompt. The summary relocation also no longer requires the header text to be the current one: a pre-0.9.6 carrier that begins at the legacy marker carries the provenance block too, so it is relocated like any other generated checkpoint. Signed-off-by: z27014 --- crates/tui/src/client/chat.rs | 189 +++++++++++++++++-- crates/tui/src/compaction.rs | 149 ++++++++++++--- crates/tui/src/compaction/last_round.rs | 35 +++- crates/tui/src/runtime_handoff.rs | 230 +++++++++++++++++++++--- 4 files changed, 538 insertions(+), 65 deletions(-) diff --git a/crates/tui/src/client/chat.rs b/crates/tui/src/client/chat.rs index 2979aa47ff..cd5a0bc7d7 100644 --- a/crates/tui/src/client/chat.rs +++ b/crates/tui/src/client/chat.rs @@ -2620,26 +2620,24 @@ fn build_chat_messages_with_reasoning( .rev() .find_map(|(index, message)| { (index > 0 - && crate::compaction::is_wire_compaction_checkpoint_message(message) - && messages[index - 1] - .content - .iter() - .any(|block| matches!(block, ContentBlock::ToolResult { .. }))) + && crate::compaction::is_generated_compaction_checkpoint(message) + && crate::runtime_handoff::carries_tool_result(&messages[index - 1])) .then_some(index) }); - let summary_target = summary_index.and_then(|summary_index| { - messages[..summary_index] - .iter() - .rposition(|message| { - crate::runtime_handoff::classify_user_turn_prompt(message) - != crate::runtime_handoff::UserTurnPromptKind::NotPrompt - }) - .or_else(|| { - messages[..summary_index] - .iter() - .position(|message| message.role.is_assistant_like()) - }) - }); + let summary_target = summary_index + .and_then(|summary_index| { + crate::runtime_handoff::compaction_anchor(messages, summary_index) + }) + .map(crate::runtime_handoff::CompactionAnchor::request_index); + if summary_index.is_some() && summary_target.is_none() { + // Degenerate saved history: the summary trails a tool result, yet no + // retained prompt or assistant message precedes it. It stays where it + // is (moving it somewhere arbitrary would be guesswork), so the request + // keeps the shape a strict template rejects. Worth a breadcrumb. + logging::warn( + "Compaction summary trails a tool result with no retained prompt or assistant to anchor it", + ); + } let wire_messages = (0..messages.len()) .filter(|index| Some(*index) != summary_index || summary_target.is_none()) .flat_map(|index| { @@ -6445,6 +6443,13 @@ mod image_block_wire_tests { const DATA_URL: &str = "data:image/png;base64,QUJD"; + /// Invariant: no `role="user"` message directly follows a `role="tool"` + /// message, with tool-call/result IDs still paired. + /// + /// Not covered here, and not claimed: compaction retains older user turns + /// verbatim, so a pass that retains two of them still emits adjacent user + /// messages, and any template that demands full alternation would reject + /// that shape. See the PR body's residual notes. #[test] fn forkguard_compaction_tool_round_has_valid_chat_wire_roles() { let mut messages: Vec = serde_json::from_value(serde_json::json!([ @@ -6544,6 +6549,66 @@ mod image_block_wire_tests { assert_eq!(restored_wire[6]["content"], "What happened next?"); } + #[test] + fn forkguard_restored_pre_fix_session_has_valid_chat_wire_roles() { + // Sessions saved before the placement fix append the topology sidecar + // to the end of the replacement history, so restore sees it after the + // summary (either order both releases produced). Request-time + // rewriting moves only the summary, so without an in-place repair the + // first request after restore is `tool → user` again — the exact 400 + // this pipeline exists to fix. + let checkpoint = crate::models::SystemPrompt::Text( + crate::compaction::build_compaction_summary_block_text("Compacted summary", ""), + ); + for topology_after_summary in [false, true] { + let mut messages: Vec = serde_json::from_value(serde_json::json!([ + {"role":"user","content":[{"type":"text","text":"Analyze the data"}]}, + {"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":"ready"}]} + ])) + .unwrap(); + let topology = persisted_topology_message(); + let summary = Message { + role: Role::User, + content: vec![ContentBlock::Text { + text: crate::compaction::build_compaction_summary_block_text( + "Compacted summary", + "", + ), + cache_control: None, + }], + }; + if topology_after_summary { + messages.push(summary); + messages.push(topology); + } else { + messages.push(topology); + messages.push(summary); + } + + let restored = crate::compaction::restore_compaction_checkpoint( + crate::runtime_handoff::project_messages_for_restore(&messages), + Some(&checkpoint), + ); + let wire = build_chat_messages(None, &restored, "gpt-4o"); + let roles: Vec<&str> = wire + .iter() + .map(|message| message["role"].as_str().unwrap()) + .collect(); + assert_eq!( + roles, + ["user", "assistant", "tool"], + "topology_after_summary={topology_after_summary}: {restored:?}" + ); + let prompt = wire[0]["content"].as_str().unwrap(); + assert!(prompt.contains("Compacted summary")); + assert!(prompt.contains("Analyze the data")); + assert!(prompt.contains("restored Agent topology checkpoint")); + assert_eq!(wire[1]["tool_calls"][0]["id"], "call_1"); + assert_eq!(wire[2]["tool_call_id"], "call_1"); + } + } + #[test] fn restored_completion_does_not_join_an_ordinary_user_turn() { let prompt: Message = serde_json::from_value(serde_json::json!({ @@ -6672,6 +6737,12 @@ mod image_block_wire_tests { .map(|message| message["role"].as_str().unwrap()) .collect(); assert_eq!(roles, ["user", "assistant", "tool"]); + assert!( + wire[0]["content"] + .as_str() + .unwrap() + .contains("Run the suite") + ); assert!( wire[0]["content"] .as_str() @@ -6704,10 +6775,84 @@ mod image_block_wire_tests { .unwrap() .contains("Run the suite") ); + // The summary belongs to the round's own prompt, not to the retained + // older turn it happens to sit next to in saved history. + assert!(wire[1]["content"].as_str().unwrap().contains("Summary")); assert_eq!(wire[2]["role"], "assistant"); assert_eq!(wire[3]["role"], "tool"); } + /// The provenance block, not the header text, decides relocation. Sessions + /// from before the header was rewritten carry the legacy marker, and their + /// first request must be reordered too. + #[test] + fn legacy_summary_carrier_moves_ahead_of_its_prompt_on_the_wire() { + let mut messages: Vec = serde_json::from_value(serde_json::json!([ + {"role":"user","content":[{"type":"text","text":"Continue from the handoff"}]}, + {"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(format!( + "{}\nold-format body", + crate::compaction::LEGACY_COMPACTION_SUMMARY_MARKER + )), + )); + + let wire = build_chat_messages(None, &messages, "gpt-4o"); + let roles: Vec<&str> = wire + .iter() + .map(|message| message["role"].as_str().unwrap()) + .collect(); + assert_eq!(roles, ["user", "assistant", "tool"]); + let prompt = wire[0]["content"].as_str().unwrap(); + assert!(prompt.contains("old-format body")); + assert!(prompt.contains("Continue from the handoff")); + assert_eq!(wire[1]["tool_calls"][0]["id"], "call_1"); + assert_eq!(wire[2]["tool_call_id"], "call_1"); + } + + /// A summary whose retained history has no prompt left anchors before the + /// first assistant message instead of staying after the tool result. A + /// history with neither anchor keeps it in place rather than inventing + /// one; this fixture is unreachable from a real save, and the wire already + /// drops the orphaned result for want of a matching tool call. + #[test] + fn summary_without_a_retained_prompt_anchors_before_the_assistant() { + let mut messages = vec![Message { + role: Role::Assistant, + content: vec![fixture_tool_use("call_1")], + }]; + messages.push(fixture_tool_result("call_1")); + 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 roles: Vec<&str> = wire + .iter() + .map(|message| message["role"].as_str().unwrap()) + .collect(); + assert_eq!(roles, ["user", "assistant", "tool"]); + assert_eq!(wire[1]["tool_calls"][0]["id"], "call_1"); + assert_eq!(wire[2]["tool_call_id"], "call_1"); + + let degenerate = vec![fixture_tool_result("call_1"), messages[2].clone()]; + let degenerate_wire = build_chat_messages(None, °enerate, "gpt-4o"); + let roles: Vec<&str> = degenerate_wire + .iter() + .map(|message| message["role"].as_str().unwrap()) + .collect(); + assert_eq!( + roles, + ["user"], + "with no anchor the summary stays put and the orphaned result is dropped" + ); + } + fn fixture_tool_use(id: &str) -> ContentBlock { ContentBlock::ToolUse { id: id.to_string(), @@ -6730,6 +6875,14 @@ mod image_block_wire_tests { } } + /// A sidecar exactly as a compaction commit persisted it: built by the one + /// production constructor, without placing it in a replacement history. + fn persisted_topology_message() -> Message { + let mut empty: Vec = Vec::new(); + crate::runtime_handoff::replace_agent_topology_checkpoint(&mut empty, &[]); + empty.remove(0) + } + fn request_with_image() -> MessageRequest { MessageRequest { model: "gpt-4o".to_string(), diff --git a/crates/tui/src/compaction.rs b/crates/tui/src/compaction.rs index bedadb5313..b5d41b77a5 100644 --- a/crates/tui/src/compaction.rs +++ b/crates/tui/src/compaction.rs @@ -296,19 +296,65 @@ pub(crate) fn compaction_checkpoint_message(prompt: &SystemPrompt) -> Message { } } +/// Whether the first text block carries the summary header, current or legacy. +fn has_compaction_summary_header(text: &str) -> bool { + // `COMPACTION_SUMMARY_MARKER` is the current header's first sentence, and + // releases before this one also committed carriers with their own body + // after that sentence, so the prefix — not the whole header — is the + // stable shape. + text.starts_with(COMPACTION_SUMMARY_MARKER) + || text.starts_with(LEGACY_COMPACTION_SUMMARY_MARKER) +} + +/// Structural recognition of the one generated checkpoint in saved history. +/// +/// The marker substring scan above stays scoped to system-prompt carriers: +/// on history it matches 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. +/// +/// Boundary: the single-block form has no provenance to check, because +/// releases before this one saved the bare summary, and a reload that failed +/// to recognize it would stack a second summary beside it. A user turn that +/// *begins* with the header is therefore still read as a carrier here. Request +/// rewriting does not share that reading — see +/// [`is_generated_compaction_checkpoint`] — so such a turn is never moved or +/// merged on the wire. #[must_use] pub(crate) fn is_compaction_checkpoint_message(message: &Message) -> bool { - user_text_of(message).is_some_and(|text| is_compaction_summary_text(&text)) + let [ + ContentBlock::Text { + text, + cache_control: None, + }, + rest @ .., + ] = message.content.as_slice() + else { + return false; + }; + message.role == Role::User + && has_compaction_summary_header(text) + && (rest.is_empty() + || matches!( + rest, + [ContentBlock::Text { + text: provenance, + cache_control: None, + }] if provenance == COMPACTION_CHECKPOINT_PROVENANCE + )) } -/// Narrow recognition for request-time reordering. The broader legacy marker -/// scan above is needed while replacing old summaries, but must not turn an -/// ordinary user quoting that marker into a movable wire checkpoint. -pub(crate) fn is_wire_compaction_checkpoint_message(message: &Message) -> bool { +/// The provenance-stamped form, and the only form request rewriting may +/// relocate or merge. A user cannot type this shape, so an ordinary turn that +/// pastes the whole summary header after a tool result keeps its position. +#[must_use] +pub(crate) fn is_generated_compaction_checkpoint(message: &Message) -> bool { let [ ContentBlock::Text { - text, cache_control: None, + .. }, ContentBlock::Text { text: provenance, @@ -318,13 +364,13 @@ pub(crate) fn is_wire_compaction_checkpoint_message(message: &Message) -> bool { else { return false; }; - message.role == Role::User - && text.starts_with(SUMMARY_HEADER) - && provenance == COMPACTION_CHECKPOINT_PROVENANCE + provenance == COMPACTION_CHECKPOINT_PROVENANCE && is_compaction_checkpoint_message(message) } -/// Replace the saved history checkpoint with the authoritative carrier while -/// keeping its position relative to later turns after a session restore. +/// Replace the saved history checkpoint with the authoritative carrier, keep +/// 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. pub(crate) fn restore_compaction_checkpoint( mut messages: Vec, checkpoint: Option<&SystemPrompt>, @@ -332,11 +378,10 @@ pub(crate) fn restore_compaction_checkpoint( let checkpoint_index = messages.iter().position(is_compaction_checkpoint_message); messages.retain(|message| !is_compaction_checkpoint_message(message)); if let Some(checkpoint) = checkpoint { - let index = checkpoint_index - .unwrap_or(messages.len()) - .min(messages.len()); + let index = checkpoint_index.unwrap_or(messages.len()); messages.insert(index, compaction_checkpoint_message(checkpoint)); } + crate::runtime_handoff::relocate_restored_compaction_topology(&mut messages); messages } @@ -1223,12 +1268,15 @@ pub(crate) fn retained_user_messages(messages: &[Message], max_tokens: usize) -> if !crate::runtime_handoff::is_user_turn_prompt(msg) { continue; } + // A header-prefixed summary saved before the provenance block existed + // is not a user turn to carry forward, but an ordinary turn that + // merely quotes the header is. + if is_compaction_checkpoint_message(msg) { + continue; + } let Some(text) = user_text_of(msg) else { continue; }; - if is_compaction_summary_text(&text) { - continue; - } let tokens = estimate_text_tokens_conservative(&text); let text = if tokens <= remaining { remaining -= tokens; @@ -1995,6 +2043,69 @@ mod tests { } } + #[test] + fn restore_keeps_a_user_turn_that_quotes_the_summary_header() { + let quoting = "why does my log say 'Another language model started to solve this problem'?"; + let text = build_compaction_summary_block_text("first handoff", ""); + let messages = vec![ + msg("user", "hi"), + msg("assistant", "hello"), + msg("user", quoting), + msg("assistant", "It is the compaction header."), + compaction_checkpoint_message(&SystemPrompt::Text(text.clone())), + ]; + + let restored = restore_compaction_checkpoint(messages, Some(&SystemPrompt::Text(text))); + + assert!( + restored + .iter() + .any(|message| user_text_of(message).as_deref() == Some(quoting)), + "the persisted carrier is the last message, not the question quoting it: {restored:?}" + ); + assert_eq!( + restored + .iter() + .filter(|message| is_compaction_checkpoint_message(message)) + .count(), + 1 + ); + } + + #[test] + fn restore_replaces_a_pre_provenance_carrier() { + let text = build_compaction_summary_block_text("legacy handoff", ""); + let persisted = Message { + role: Role::User, + content: vec![ContentBlock::Text { + text: text.clone(), + cache_control: None, + }], + }; + assert!( + !is_generated_compaction_checkpoint(&persisted), + "a bare header-prefixed summary carries no provenance" + ); + let messages = vec![ + msg("user", "Run the suite."), + msg("assistant", "Rerunning."), + persisted, + ]; + + let restored = restore_compaction_checkpoint(messages, Some(&SystemPrompt::Text(text))); + + assert_eq!(restored.len(), 3); + assert_eq!( + restored + .iter() + .filter(|message| is_compaction_checkpoint_message(message)) + .count(), + 1, + "the saved single-block summary must be replaced, not stacked" + ); + assert!(is_generated_compaction_checkpoint(restored.last().unwrap())); + } + #[tokio::test] async fn compaction_commits_summary_and_retains_recent_user_messages() { let messages = vec![ @@ -2082,9 +2193,7 @@ mod tests { }) })); assert!(is_compaction_checkpoint_message(retained.last().unwrap())); - assert!(is_wire_compaction_checkpoint_message( - retained.last().unwrap() - )); + assert!(is_generated_compaction_checkpoint(retained.last().unwrap())); let ContentBlock::Text { text: body, .. } = &retained.last().unwrap().content[0] else { panic!("compaction checkpoint must begin with its summary"); }; diff --git a/crates/tui/src/compaction/last_round.rs b/crates/tui/src/compaction/last_round.rs index 2b0119a908..6330a02bd5 100644 --- a/crates/tui/src/compaction/last_round.rs +++ b/crates/tui/src/compaction/last_round.rs @@ -171,7 +171,8 @@ pub(crate) fn last_round_range(messages: &[Message]) -> (usize, usize) { (start, messages.len()) } -/// How many messages of the open round sit in `messages` before a checkpoint. +/// How many round messages the open round kept in a history carrying a +/// checkpoint; `None` when there is no checkpoint to report on. #[must_use] pub fn last_round_kept_count(messages: &[Message]) -> Option { messages @@ -604,6 +605,38 @@ mod tests { assert!(error.to_string().contains("anchor"), "{error}"); } + #[test] + fn forkguard_marker_quoted_by_a_user_turn_survives_recompaction() { + let quoting = "why does my log say 'Another language model started to solve this problem'?"; + let mut messages = vec![ + msg("user", "Run the suite now."), + tool_use("live", "Bash", json!({"command": "cargo test"})), + tool_result("live", "ok"), + checkpoint("suite finished"), + ]; + crate::runtime_handoff::replace_agent_topology_checkpoint(&mut messages, &[]); + messages.push(msg("assistant", "It is the compaction header.")); + messages.push(msg("user", quoting)); + + let next = crate::compaction::build_compaction_summary_block_text("second summary", ""); + let replacement = build_replacement_history(&messages, &next, None) + .expect("a quoting user turn must not break the round"); + + assert!( + replacement + .iter() + .any(|message| user_text_of(message).as_deref() == Some(quoting)), + "the quoting turn is user intent, not the checkpoint: {replacement:?}" + ); + assert_eq!( + replacement + .iter() + .filter(|message| is_compaction_checkpoint_message(message)) + .count(), + 1 + ); + } + #[test] fn last_round_starts_at_the_latest_plain_user_message() { let messages = vec![ diff --git a/crates/tui/src/runtime_handoff.rs b/crates/tui/src/runtime_handoff.rs index 70644d01b8..918e8b5d0e 100644 --- a/crates/tui/src/runtime_handoff.rs +++ b/crates/tui/src/runtime_handoff.rs @@ -452,6 +452,72 @@ pub(crate) fn is_agent_topology_checkpoint(message: &Message) -> bool { && text.ends_with(AGENT_TOPOLOGY_EVENT_SUFFIX) } +/// True when a message carries a tool result, which strict paired chat +/// templates treat as the user's half of the tool exchange: the next wire +/// message must not be `role="user"`. +pub(crate) fn carries_tool_result(message: &Message) -> bool { + message.content.iter().any(|block| { + matches!( + block, + ContentBlock::ToolResult { .. } + | ContentBlock::ToolSearchToolResult { .. } + | ContentBlock::CodeExecutionToolResult { .. } + ) + }) +} + +/// The boundary a compaction-owned runtime message belongs to inside +/// `messages[..before]`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum CompactionAnchor { + /// The latest genuine user prompt. The message belongs after it, before + /// that round's assistant/tool chain, and merges into it on the wire. + Prompt(usize), + /// No prompt survives in the window; the message belongs before the first + /// assistant message. + Assistant(usize), +} + +impl CompactionAnchor { + /// Saved position: after the prompt, before the assistant chain. + pub(crate) fn placement_index(self) -> usize { + match self { + CompactionAnchor::Prompt(index) => index + 1, + CompactionAnchor::Assistant(index) => index, + } + } + + /// Request position: directly before the message a moved summary merges + /// into, so the pair reaches the wire as one user turn. + pub(crate) fn request_index(self) -> usize { + match self { + CompactionAnchor::Prompt(index) | CompactionAnchor::Assistant(index) => index, + } + } +} + +/// Locate the boundary a compaction-owned runtime message belongs to: the +/// latest genuine user prompt in `messages[..before]`, or the first assistant +/// message when that window retains no prompt. `None` means the window offers +/// neither, and callers fall back to "append at the end" / "leave in place". +/// +/// One definition for save-time placement, restore repair, and request-time +/// summary relocation; the three used to be near-copies that had already +/// drifted on which messages count as a prompt. +pub(crate) fn compaction_anchor(messages: &[Message], before: usize) -> Option { + let window = messages.get(..before)?; + window + .iter() + .rposition(|message| classify_user_turn_prompt(message) != UserTurnPromptKind::NotPrompt) + .map(CompactionAnchor::Prompt) + .or_else(|| { + window + .iter() + .position(|message| message.role.is_assistant_like()) + .map(CompactionAnchor::Assistant) + }) +} + /// Install one bounded, typed Agent-topology sidecar after replacement /// compaction. A current empty topology is still meaningful: it overrides a /// narrative summary or old runtime event that says an Agent remains live. @@ -466,36 +532,18 @@ pub(crate) fn replace_agent_topology_checkpoint( messages: &mut Vec, snapshots: &[SubAgentResult], ) { - messages.retain(|message| !is_agent_topology_checkpoint(message)); - let ends_with_tool_result = messages.last().is_some_and(|message| { - message.content.iter().any(|block| { - matches!( - block, - ContentBlock::ToolResult { .. } - | ContentBlock::ToolSearchToolResult { .. } - | ContentBlock::CodeExecutionToolResult { .. } - ) - }) + // Both forms: a restored carrier is the projection of a sidecar a previous + // save already wrote, and keeping it beside the fresh one would persist a + // superseded duplicate that no later replay removes. + messages.retain(|message| { + !is_agent_topology_checkpoint(message) && !is_restored_agent_topology_checkpoint(message) }); + let ends_with_tool_result = messages.last().is_some_and(carries_tool_result); let ends_with_compaction_summary = messages .last() - .is_some_and(crate::compaction::is_wire_compaction_checkpoint_message); + .is_some_and(crate::compaction::is_generated_compaction_checkpoint); let position = if ends_with_tool_result || ends_with_compaction_summary { - messages - .iter() - .rposition(|message| { - !crate::compaction::is_wire_compaction_checkpoint_message(message) - && classify_user_turn_prompt(message) != UserTurnPromptKind::NotPrompt - }) - .map_or_else( - || { - messages - .iter() - .position(|message| message.role.is_assistant_like()) - .unwrap_or(0) - }, - |index| index + 1, - ) + compaction_anchor(messages, messages.len()).map_or(0, CompactionAnchor::placement_index) } else { messages.len() }; @@ -998,6 +1046,77 @@ pub(crate) fn is_restored_agent_topology_checkpoint(message: &Message) -> bool { .is_some_and(|display| display.starts_with(RESTORED_TOPOLOGY_HEADER)) } +fn is_compaction_topology_carrier(message: &Message) -> bool { + is_agent_topology_checkpoint(message) || is_restored_agent_topology_checkpoint(message) +} + +/// Where a restored topology carrier should sit, or `None` when it is already +/// there. The placement the fix writes is "directly after the genuine prompt +/// its summary is merged into, before that round's assistant/tool chain", so a +/// carrier is misplaced when it trails its own summary or directly follows a +/// tool result — the two shapes the pre-fix releases saved. +fn restored_topology_anchor(messages: &[Message], index: usize) -> Option { + let summary_before = messages[..index] + .iter() + .rposition(crate::compaction::is_compaction_checkpoint_message); + let follows_tool_result = index > 0 && carries_tool_result(&messages[index - 1]); + if summary_before.is_none() && !follows_tool_result { + return None; + } + let before = summary_before + .or_else(|| { + messages[index + 1..] + .iter() + .position(crate::compaction::is_compaction_checkpoint_message) + .map(|offset| index + 1 + offset) + }) + .unwrap_or(messages.len()); + let anchor = compaction_anchor(messages, before)?.placement_index(); + (anchor < index).then_some(anchor) +} + +/// Repair the topology sidecar of a session saved before the placement fix. +/// +/// Those releases appended the sidecar to the end of the replacement history, +/// so a restored session carries it after its own summary or directly after a +/// tool result. Request-time rewriting only moves the summary, which leaves a +/// `tool → user` wire that strict paired chat templates reject with the same +/// 400 this pipeline exists to fix. Moving the carrier to its placement anchor +/// on restore repairs the session in place instead of asking for a new one. +/// Sessions saved by the fix are already at that anchor and stay untouched. +pub(crate) fn relocate_restored_compaction_topology(messages: &mut Vec) { + let plan: Vec> = messages + .iter() + .enumerate() + .map(|(index, message)| { + is_compaction_topology_carrier(message) + .then(|| restored_topology_anchor(messages, index)) + .flatten() + }) + .collect(); + if plan.iter().all(Option::is_none) { + return; + } + let mut slots: Vec> = std::mem::take(messages).into_iter().map(Some).collect(); + let mut rebuilt = Vec::with_capacity(slots.len()); + for index in 0..=slots.len() { + // `plan` holds insertion positions, matching the save-time + // `Vec::insert` the anchor was defined for, so a relocated carrier is + // emitted before the message that currently occupies that position. + for (from, anchor) in plan.iter().enumerate() { + if *anchor == Some(index) + && let Some(message) = slots[from].take() + { + rebuilt.push(message); + } + } + if let Some(message) = slots.get_mut(index).and_then(Option::take) { + rebuilt.push(message); + } + } + *messages = rebuilt; +} + /// Classification used when locating a user-authored turn in the session log. /// /// Runtime and tool messages are skipped because their provider-compatible @@ -1088,6 +1207,10 @@ pub fn is_user_turn_prompt(message: &Message) -> bool { fn is_runtime_owned_user_message(message: &Message) -> bool { restored_subagent_checkpoint_display(message).is_some() || has_non_authoritative_turn_provenance(message) + // The compaction checkpoint is engine-written history, so `/edit` must + // not treat it as the turn to truncate at: doing so deletes the summary + // the session is built on. Structure decides, not the marker substring. + || crate::compaction::is_compaction_checkpoint_message(message) } /// Return engine-owned metadata in either the current trailing shape or the @@ -1349,6 +1472,61 @@ mod tests { assert!(checkpoint.contains("all_prior_agent_lifecycle_claims")); } + #[test] + fn compaction_checkpoint_is_never_the_edit_target() { + let prompt = Message { + role: Role::User, + content: vec![ContentBlock::Text { + text: "Ship the release".to_string(), + cache_control: None, + }], + }; + let checkpoint = + crate::compaction::compaction_checkpoint_message(&crate::models::SystemPrompt::Text( + crate::compaction::build_compaction_summary_block_text("handoff", ""), + )); + assert_eq!( + classify_user_turn_prompt(&checkpoint), + UserTurnPromptKind::NotPrompt, + "the engine-written summary is not a user turn" + ); + assert!(!is_user_turn_prompt(&checkpoint)); + assert_eq!( + edit_last_turn_target(&[prompt, checkpoint]), + EditLastTurnTarget::Editable(0), + "/edit must truncate at the prompt, not delete the summary" + ); + } + + #[test] + fn replay_replaces_a_restored_topology_carrier_instead_of_stacking() { + let mut messages = vec![Message { + role: Role::User, + content: vec![ContentBlock::Text { + text: "Ship the release".to_string(), + cache_control: None, + }], + }]; + replace_agent_topology_checkpoint(&mut messages, &[]); + + let mut recompacted = project_messages_for_restore(&messages); + assert!(is_restored_agent_topology_checkpoint(&recompacted[1])); + replace_agent_topology_checkpoint(&mut recompacted, &[]); + + assert_eq!( + recompacted + .iter() + .filter(|message| { + is_agent_topology_checkpoint(message) + || is_restored_agent_topology_checkpoint(message) + }) + .count(), + 1, + "a superseded restored carrier must not survive beside the fresh one: {recompacted:?}" + ); + assert_eq!(recompacted.len(), 2); + } + #[test] fn restore_projection_replaces_completion_control_plane_and_is_idempotent() { let user_task = Message {