diff --git a/docs/user/tui-and-sessions.md b/docs/user/tui-and-sessions.md index 3c01059..0175f28 100644 --- a/docs/user/tui-and-sessions.md +++ b/docs/user/tui-and-sessions.md @@ -80,7 +80,7 @@ Press `F3` to open the read-only transcript navigator, even while a response is | `Ctrl+Up` / `Ctrl+Down` | Select the previous / next User prompt, switching to the User filter while retaining the query | | `Enter` | Reveal and highlight the selected rendered block in the transcript | | `Esc` / `F3` | Close without changing transcript scroll position, editor text, or attachments | -| Type `/branch`, then `Enter` | While idle, open the backend-approved text prompt checkout chooser | +| Type `/branch`, then `Enter` | While idle, open the backend-approved conversation checkpoint chooser | Navigator queries are limited to 4,096 UTF-8 bytes. Oversized pastes keep a bounded whole-grapheme prefix; pasted newlines and tabs are ignored rather than activating navigator controls. @@ -88,21 +88,21 @@ Revealing a Thought block temporarily shows it even when reasoning is hidden. Br The navigator searches only the currently displayed or replayed history, not a compacted archive or undelivered messages in the pending queue. Tool searches include display text such as the title, script, and output; media labels are searchable, but binary media payloads are not. Block identities are local and ephemeral, not durable addresses for forking. This is navigation only: it does not fork a session, write history, cancel a turn, or send a prompt. -### Edit a previous prompt in a new session +### Branch from a text checkpoint in a new session -While idle, type the exact local command `/branch`, or open `/transcript` (`F3`), type `/branch` in its search field, and press Enter. The separate checkout chooser lists only text prompts approved by the backend. Archived prompts are labeled `[archived]`; this list is authoritative, not inferred from the visible transcript. Use Up/Down to select a prompt and Enter to prepare an edit. Unsupported agents or ineligible prompts produce an error without changing the source session. +While idle, type the exact local command `/branch`, or open `/transcript` (`F3`), type `/branch` in its search field, and press Enter. The separate checkout chooser lists only conversation checkpoints approved by the backend, labeled `[user]`, `[assistant]`, or `[tool]`. Archived checkpoints also show `[archived]`; this list is authoritative, not inferred from the visible transcript. Press `0` for All, `1` for User, `2` for Assistant, or `3` for Tool. Changing the filter selects its first match. Use Up/Down to select a checkpoint and Enter to prepare a draft; an empty filter has nothing to prepare. Filtering never changes the backend address used for checkout. Unsupported agents or ineligible checkpoints produce an error without changing the source session. `/transcript` remains a display-only navigator: its local block positions are not checkout addresses. > Only conversation context changes. Filesystem changes, running processes, and external effects are not rolled back. -The TUI displays the backend's conversation prefix and puts the original prompt text in a **provisional prompt checkout** editor. Your source transcript, unsent editor draft, attachments, and model/configuration state are parked, not discarded. Edit the text, use Shift+Enter for a newline, then Enter to create and activate a new persisted session. No branch is created by merely browsing or preparing an edit. The edited prompt is persisted by branch submission itself; the TUI does not send it a second time as an ordinary prompt. +The TUI displays the backend's safe conversation prefix in a **provisional prompt checkout**. Selecting a user prompt starts **before** that prompt and prefills the editor with its unchanged text for replacement. Selecting an assistant message or tool result starts **after** its safe backend-approved boundary and opens an empty editor for a new user continuation; the chooser preview is not inserted as user text. Your source transcript, unsent editor draft, attachments, and model/configuration state are parked, not discarded. Edit or enter text, use Shift+Enter for a newline, then Enter to create and activate a new persisted session. Empty or whitespace-only drafts cannot be submitted. No branch is created by merely browsing or preparing an edit. The edited prompt is persisted by branch submission itself; the TUI does not send it a second time as an ordinary prompt. Press Esc before submission to abandon the checkout and restore the parked source view, draft, attachments, and configuration. Esc also cancels a pending list or prepare request; late responses cannot replace a newer view. Once submission is in progress, wait for its result: Esc cannot undo a committed branch. A failed submit keeps the provisional draft available. Retry with the same text to recover a child if the response was lost. Once a submission reaches the backend, its checkout token is bound to that exact text; to submit a different edit, abandon and prepare a new checkout. Checkout edits are text-only. Adding image or audio attachments is rejected; attachments already in the parked source draft remain intact. While the checkout chooser or provisional editor is active, model/configuration changes, session switching, ordinary sends, and steering are disabled. Slash-command text in the provisional editor is edited prompt text, not a local command. -The child retains the conversation strictly before the selected prompt, including its original bootstrap context. Archived prompts use validated pre-compaction history, never a later summary as a substitute for missing context. Prompts with unsupported content, unresolved tool calls in their prefix, or unreconstructable legacy context are not eligible. +The child retains the safe prefix before the selected user prompt or after the selected assistant/tool checkpoint, including its original bootstrap context. Committed assistant text can be selected whether it is an intermediate or final answer; reasoning-only summaries are not checkpoints. A tool-result checkpoint must close every pending call in its batch, including parallel calls. Selecting a partial result never advances past it to collect later answers, and an assistant item containing text plus tool calls is never split. Archived checkpoints use validated pre-compaction history, never a later summary as a substitute for missing context. Checkpoints with unsupported content, unresolved tool calls in their prefix, or unreconstructable legacy context are not eligible. -An unsubmitted checkout becomes stale when its source conversation or configuration changes, including compaction, or when the backend restarts. Abandon it and list prompts again. Committed submissions survive restart: retrying the same checkout and text finds the same child without generating a second response. Errors after a durable commit identify the child so it remains discoverable even if activation or response delivery failed. +An unsubmitted checkout becomes stale when its source conversation or configuration changes, including compaction, or when the backend restarts. Abandon it and list checkpoints again. Committed submissions survive restart: retrying the same checkout and text finds the same child without generating a second response. Errors after a durable commit identify the child so it remains discoverable even if activation or response delivery failed. After successful submission, the source remains loaded and unchanged. Use `/sessions` to return to it. If the child is cancelled before execution starts, its committed history remains available but its connection can close to prevent the cancelled prompt from running later. Select the source, then the child in `/sessions` to reload it without rerunning that prompt. Checkout does not restore files, stop processes, reverse tool calls, or undo any other external effect. diff --git a/src/protocols/acp/prompt_branches.rs b/src/protocols/acp/prompt_branches.rs index f038804..92aada0 100644 --- a/src/protocols/acp/prompt_branches.rs +++ b/src/protocols/acp/prompt_branches.rs @@ -1,4 +1,4 @@ -//! Kit-private text-prompt checkout protocol. Addresses are issued by the backend; +//! Kit-private conversation checkout protocol. Addresses are issued by the backend; //! neither display positions nor provider item identifiers are branch authority. use agentkit_acp::v2::wire; @@ -16,10 +16,23 @@ pub(crate) struct ListPromptBranchesResponse { pub boundaries: Vec, } +/// Missing roles on the legacy private wire describe before-user checkouts. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub(crate) enum PromptRole { + #[default] + User, + Assistant, + Tool, +} + #[derive(Debug, Clone, Serialize, Deserialize)] pub(crate) struct PromptBoundary { pub address: String, + /// Chooser preview, not the prepared editor contents. pub text: String, + #[serde(default)] + pub role: PromptRole, pub historical: bool, } @@ -113,7 +126,9 @@ struct ApprovedBoundary { parent_session_id: String, provenance: branch::Boundary, state: Arc>, - text: String, + role: PromptRole, + selection_identity: blake3::Hash, + original_text: String, } #[derive(Clone)] @@ -240,19 +255,23 @@ impl PromptCheckouts { } let state = Arc::new(state.clone()); for (index, item) in state.iter().enumerate() { - if item.kind != ItemKind::User { - continue; - } - let Ok(text) = text_prompt(item) else { + let Some((role, text, original_text)) = checkout_candidate(item) else { continue; }; - let prefix = &state[..index]; - if prefix.is_empty() || crate::transcript::has_unanswered_tool_calls(prefix) { + // User edits replace the selected prompt; continuations retain + // the entire selected assistant/result item, never a partial batch. + let prefix_len = index + usize::from(role != PromptRole::User); + let prefix = &state[..prefix_len]; + if prefix.is_empty() || crate::transcript::validate_checkout_prefix(prefix).is_err() + { continue; } - let identity = - serde_json::to_vec(&state[..=index]).map_err(|error| error.to_string())?; - if !seen.insert(blake3::hash(&identity)) { + // Distinct selections can retain the same prefix (an assistant + // answer and the following user prompt). Bind role AND selection. + let identity = serde_json::to_vec(&(role, &state[..=index])) + .map_err(|error| error.to_string())?; + let selection_identity = blake3::hash(&identity); + if !seen.insert(selection_identity) { continue; } let provenance = branch::Boundary::new(state_index, prefix)?; @@ -260,7 +279,9 @@ impl PromptCheckouts { .boundaries .iter() .find_map(|(address, existing)| { - (existing.provenance == provenance && existing.text == text) + (existing.provenance == provenance + && existing.role == role + && existing.selection_identity == selection_identity) .then(|| address.clone()) }) .unwrap_or_else(crate::session::new_id); @@ -269,12 +290,15 @@ impl PromptCheckouts { parent_session_id: session_id.into(), provenance, state: Arc::clone(&state), - text: text.clone(), + role, + selection_identity, + original_text, }; self.boundaries.insert(address.clone(), boundary); result.push(PromptBoundary { address, text, + role, historical: state_index + 1 != states.len(), }); } @@ -299,7 +323,7 @@ impl PromptCheckouts { .validate(transcript, checkout_revision, selection, reasoning)?; let checkout = PreparedCheckout { token: crate::session::new_id(), - original_text: boundary.text.clone(), + original_text: boundary.original_text.clone(), prefix: boundary.state[..boundary.provenance.prefix_len].to_vec(), selection: selection.clone(), reasoning, @@ -331,6 +355,48 @@ impl PromptCheckouts { } } +/// Only selection-specific restrictions belong here. The ordered prefix +/// validator checks inherited history without imposing text-only user input. +fn checkout_candidate(item: &Item) -> Option<(PromptRole, String, String)> { + match item.kind { + ItemKind::User => { + let text = text_prompt(item).ok()?; + Some((PromptRole::User, text.clone(), text)) + } + ItemKind::Assistant => { + let mut text = String::new(); + for part in &item.parts { + match part { + Part::Text(part) => text.push_str(&part.text), + Part::Reasoning(_) | Part::ToolCall(_) => {} + // Do not offer a selected item whose visible content would + // be dropped by the sanitizer or unsupported by adapters. + _ => return None, + } + } + // A reasoning display summary is not portable assistant text. + (!text.trim().is_empty()).then_some((PromptRole::Assistant, text, String::new())) + } + ItemKind::Tool => { + let mut previews = Vec::new(); + for part in &item.parts { + let Part::ToolResult(result) = part else { + return None; + }; + // Preview only: never serialize potentially large/binary tool + // outputs into the chooser. The retained snapshot is unchanged. + let output = match &result.output { + agentkit_core::ToolOutput::Text(text) => text.chars().take(240).collect(), + _ => "[non-text result]".to_string(), + }; + previews.push(format!("{}: {output}", result.call_id)); + } + (!previews.is_empty()).then_some((PromptRole::Tool, previews.join("\n"), String::new())) + } + _ => None, + } +} + fn text_prompt(item: &Item) -> Result { if item.kind != ItemKind::User || item.parts.is_empty() { return Err("checkout supports text-only user prompts".into()); @@ -408,11 +474,11 @@ mod tests { let before = source.clone(); let mut checkouts = PromptCheckouts::default(); let boundaries = list(&mut checkouts, &source, std::slice::from_ref(&source)); - assert_eq!(boundaries.boundaries.len(), 2); + assert_eq!(boundaries.boundaries.len(), 4); for (index, prefix_len) in [2, 4].into_iter().enumerate() { let prepared = checkouts .prepare( - &boundaries.boundaries[index].address, + &boundaries.boundaries[index * 2].address, &source, 0, &selection(), @@ -440,6 +506,228 @@ mod tests { assert_eq!(source, before); } + #[test] + fn assistant_continuations_keep_exact_inclusive_prefix_and_empty_draft() { + // Neither intermediate nor final committed text needs a finish-reason + // marker. The following user selection retains the same prefix but has + // distinct authority and still prefills its original prompt. + let source = conversation(); + let mut checkouts = PromptCheckouts::default(); + let listed = list(&mut checkouts, &source, std::slice::from_ref(&source)); + for (boundary_index, prefix_len) in [(1, 4), (3, 6)] { + let boundary = &listed.boundaries[boundary_index]; + assert_eq!(boundary.role, PromptRole::Assistant); + assert!(!boundary.text.is_empty()); + let prepared = checkouts + .prepare(&boundary.address, &source, 0, &selection(), None) + .unwrap(); + assert_eq!(prepared.prefix, source[..prefix_len]); + assert!(prepared.original_text.is_empty()); + assert!(prepared.fork(" \n").is_err()); + let fork = prepared.fork("continue here").unwrap(); + assert_eq!(&fork.transcript[1..prefix_len], &source[1..prefix_len]); + assert_eq!(fork.transcript.len(), prefix_len + 1); + assert_eq!( + text_prompt(fork.transcript.last().unwrap()).unwrap(), + "continue here" + ); + let metadata = branch::BranchMetadata::read(&fork.transcript) + .unwrap() + .unwrap(); + assert_eq!(metadata.boundary.prefix_len, prefix_len); + assert!( + checkouts + .prepare(&boundary.address, &source, 1, &selection(), None) + .is_err() + ); + } + assert_ne!(listed.boundaries[1].address, listed.boundaries[2].address); + let again = list(&mut checkouts, &source, std::slice::from_ref(&source)); + for (old, new) in listed.boundaries.iter().zip(&again.boundaries) { + assert_eq!(old.address, new.address); + } + } + + fn tool_result(id: &str) -> Item { + Item::new( + ItemKind::Tool, + vec![Part::ToolResult(agentkit_core::ToolResultPart::success( + id, + agentkit_core::ToolOutput::text(format!("result {id}")), + ))], + ) + } + + fn parallel_conversation() -> Vec { + vec![ + Item::text(ItemKind::System, "bootstrap"), + Item::text(ItemKind::User, "run tools"), + Item::new( + ItemKind::Assistant, + vec![ + Part::text("calling both tools"), + Part::ToolCall(ToolCallPart::new("a", "tool", json!({}))), + Part::ToolCall(ToolCallPart::new("b", "tool", json!({}))), + ], + ), + tool_result("a"), + tool_result("b"), + Item::text(ItemKind::Assistant, "after tools"), + ] + } + + #[test] + fn only_last_parallel_result_is_a_boundary_in_either_order() { + for reverse in [false, true] { + let mut source = parallel_conversation(); + if reverse { + source.swap(3, 4); + } + let mut checkouts = PromptCheckouts::default(); + let listed = list(&mut checkouts, &source, std::slice::from_ref(&source)); + assert_eq!( + listed.boundaries.iter().map(|b| b.role).collect::>(), + [PromptRole::User, PromptRole::Tool, PromptRole::Assistant] + ); + let boundary = &listed.boundaries[1]; + let prepared = checkouts + .prepare(&boundary.address, &source, 0, &selection(), None) + .unwrap(); + assert_eq!(prepared.prefix, source[..5]); + assert!(prepared.original_text.is_empty()); + let fork = prepared.fork("continue after tools").unwrap(); + assert_eq!(&fork.transcript[1..5], &source[1..5]); + assert_eq!(fork.transcript.len(), 6); + // Listing a still-open batch must not move the selected partial + // result forward or split the text+calls assistant item. + for end in [3, 4] { + let partial = &source[..end]; + let listed = list(&mut checkouts, partial, &[partial.to_vec()]); + assert_eq!(listed.boundaries.len(), 1); + assert_eq!(listed.boundaries[0].role, PromptRole::User); + } + } + } + + #[test] + fn invalid_ordered_history_never_offers_later_candidates() { + let source = parallel_conversation(); + let malformed = [ + vec![source[2].clone(), source[3].clone()], // missing result + vec![source[3].clone()], // orphan / before call + vec![source[3].clone(), source[2].clone(), source[4].clone()], + vec![ + source[2].clone(), + source[3].clone(), + source[3].clone(), + source[4].clone(), + ], + vec![ + source[2].clone(), + source[2].clone(), + source[3].clone(), + source[4].clone(), + ], + vec![ + source[2].clone(), + Item::text(ItemKind::User, "interrupt"), + source[3].clone(), + source[4].clone(), + ], + ]; + for middle in malformed { + let mut invalid = source[..2].to_vec(); + invalid.extend(middle); + invalid.push(Item::text(ItemKind::Assistant, "unsafe answer")); + invalid.push(Item::text(ItemKind::User, "unsafe prompt")); + let listed = list( + &mut PromptCheckouts::default(), + &invalid, + std::slice::from_ref(&invalid), + ); + assert_eq!(listed.boundaries.len(), 1); + assert_eq!(listed.boundaries[0].text, "run tools"); + } + } + + #[test] + fn reasoning_only_is_not_a_candidate_but_inherited_reasoning_and_user_media_survive() { + let mut source = conversation()[..3].to_vec(); + source.push(Item::new( + ItemKind::Assistant, + vec![Part::Reasoning(agentkit_core::ReasoningPart::summary( + "displayed thought, not assistant text", + ))], + )); + source.push(Item::new( + ItemKind::User, + vec![Part::media( + Modality::Image, + "image/png", + DataRef::InlineBytes(vec![1]), + )], + )); + source.push(Item::new( + ItemKind::Assistant, + vec![ + Part::Reasoning(agentkit_core::ReasoningPart::summary("inherited thought")), + Part::text("portable answer"), + ], + )); + source.push(Item::text(ItemKind::User, "next prompt")); + let mut checkouts = PromptCheckouts::default(); + let listed = list(&mut checkouts, &source, std::slice::from_ref(&source)); + assert_eq!(listed.boundaries.len(), 3); + for boundary in &listed.boundaries[1..] { + let prepared = checkouts + .prepare(&boundary.address, &source, 0, &selection(), None) + .unwrap(); + assert_eq!(prepared.prefix, source[..6]); + let fork = prepared.fork("edited").unwrap(); + assert_eq!(&fork.transcript[1..6], &source[1..6]); + } + source[5].parts.push(Part::media( + Modality::Image, + "image/png", + DataRef::InlineBytes(vec![1]), + )); + assert!(checkout_candidate(&source[5]).is_none()); + } + + #[test] + fn role_wire_defaults_legacy_and_writes_current_shape() { + let legacy = json!({"address":"opaque", "text":"preview", "historical":false}); + let parsed: PromptBoundary = serde_json::from_value(legacy.clone()).unwrap(); + assert_eq!(parsed.role, PromptRole::User); + assert_eq!(serde_json::to_value(parsed).unwrap()["role"], "user"); + for (name, role) in [ + ("user", PromptRole::User), + ("assistant", PromptRole::Assistant), + ("tool", PromptRole::Tool), + ] { + let mut current = legacy.clone(); + current["role"] = json!(name); + let parsed: PromptBoundary = serde_json::from_value(current.clone()).unwrap(); + assert_eq!(parsed.role, role); + assert_eq!(serde_json::to_value(parsed).unwrap(), current); + } + for malformed in [ + json!(null), + json!("Assistant"), + json!("system"), + json!(0), + json!({}), + json!([]), + ] { + let mut invalid = legacy.clone(); + invalid["role"] = malformed; + assert!(serde_json::from_value::(invalid).is_err()); + } + assert!(serde_json::from_str::( + r#"{"address":"opaque","text":"preview","historical":false,"role":"user","role":"tool"}"# + ).is_err()); + } + #[test] fn historical_checkout_never_substitutes_future_summary() { let historical = conversation(); @@ -479,6 +767,81 @@ mod tests { assert!(!now.historical); } + #[test] + fn archived_assistant_and_tool_boundaries_keep_the_selected_snapshot() { + let historical = parallel_conversation(); + let current = vec![ + historical[0].clone(), + Item::text(ItemKind::Developer, "FUTURE SUMMARY"), + Item::text(ItemKind::User, "future prompt"), + ]; + let states = vec![historical.clone(), current.clone()]; + let mut checkouts = PromptCheckouts::default(); + let listed = list(&mut checkouts, ¤t, &states); + for (role, prefix_len) in [(PromptRole::Tool, 5), (PromptRole::Assistant, 6)] { + let boundary = listed.boundaries.iter().find(|b| b.role == role).unwrap(); + assert!(boundary.historical); + let prepared = checkouts + .prepare(&boundary.address, ¤t, 0, &selection(), None) + .unwrap(); + assert_eq!(prepared.prefix, historical[..prefix_len]); + assert!(prepared.original_text.is_empty()); + assert_eq!(prepared.boundary.provenance.state_index, 0); + assert!( + !serde_json::to_string(&prepared.fork("new future").unwrap().transcript) + .unwrap() + .contains("FUTURE SUMMARY") + ); + } + // An unchanged selection retained across snapshots appears only as a + // current point; source authority still binds the exact current state. + let mut extended = historical.clone(); + extended.push(Item::text(ItemKind::User, "next")); + let listed = list(&mut checkouts, &extended, &[historical, extended.clone()]); + assert_eq!(listed.boundaries.len(), 4); + assert!( + listed + .boundaries + .iter() + .all(|boundary| !boundary.historical) + ); + } + + #[test] + fn inclusive_tool_fork_uses_shared_provider_sanitizer_without_mutating_snapshot() { + let mut source = parallel_conversation(); + let Part::ToolCall(call) = &mut source[2].parts[1] else { + unreachable!() + }; + call.metadata.insert( + "openai.responses.continuation.v1".into(), + json!({"id":"source"}), + ); + call.metadata.insert("preserved".into(), json!(true)); + let mut checkouts = PromptCheckouts::default(); + let listed = list(&mut checkouts, &source, std::slice::from_ref(&source)); + let boundary = listed + .boundaries + .iter() + .find(|b| b.role == PromptRole::Tool) + .unwrap(); + let prepared = checkouts + .prepare(&boundary.address, &source, 0, &selection(), None) + .unwrap(); + let fork = prepared.fork("continue").unwrap(); + let Part::ToolCall(call) = &fork.transcript[2].parts[1] else { + unreachable!() + }; + assert!( + !call + .metadata + .contains_key("openai.responses.continuation.v1") + ); + assert_eq!(call.metadata["preserved"], true); + assert_eq!(prepared.prefix, source[..5]); + assert_eq!(prepared.boundary.state.as_slice(), source); + } + #[test] fn read_only_lists_and_prepares_do_not_stale_existing_tokens() { let source = conversation(); @@ -497,7 +860,7 @@ mod tests { .checkout(&prepared.token, &source, 0, &selection(), None) .is_ok() ); - assert_eq!(checkouts.boundaries.len(), 2); + assert_eq!(checkouts.boundaries.len(), 4); // Restart creates a fresh actor-local authority regardless of Item.id. assert!( PromptCheckouts::default() diff --git a/src/protocols/acp/v2.rs b/src/protocols/acp/v2.rs index 2f35177..bb95442 100644 --- a/src/protocols/acp/v2.rs +++ b/src/protocols/acp/v2.rs @@ -6118,7 +6118,7 @@ mod tests { }) .block_task() .await?; - assert_eq!(listed.boundaries.len(), 1); + assert_eq!(listed.boundaries.len(), 2); let address = listed.boundaries[0].address.clone(); // An integration-rejected injection must release the admission // tracker, so a subsequent settled checkout still succeeds. diff --git a/src/session/branch.rs b/src/session/branch.rs index 208b09d..a730d3e 100644 --- a/src/session/branch.rs +++ b/src/session/branch.rs @@ -15,7 +15,8 @@ const VERSION: u32 = 1; pub(crate) struct Boundary { /// Index in `load_history`, including states preceding compaction. pub state_index: usize, - /// Number of items retained before the selected prompt. + /// Number of items in the retained prefix (exclusive for a selected user + /// prompt, inclusive for a selected assistant answer or closed tool batch). pub prefix_len: usize, /// BLAKE3 of the original parent prefix, including its metadata/timestamps. pub prefix_hash: String, diff --git a/src/transcript.rs b/src/transcript.rs index 588530e..781517d 100644 --- a/src/transcript.rs +++ b/src/transcript.rs @@ -91,6 +91,110 @@ pub(crate) fn sanitize_forked_transcript(transcript: &mut [Item]) { } } +/// Validates the ordered tool protocol at a checkout boundary without repair. +/// +/// Each assistant item with calls opens one batch; only tool results may follow until the +/// batch is complete. Parallel results may arrive in any order, but call IDs +/// cannot be reused anywhere in the prefix. An empty prefix is valid. +/// +/// Also rejects role/part combinations unsupported by both the Completions and +/// Responses adapters. This is their union, not a promise that every provider +/// can encode the prefix: user files and reasoning remain Completions-compatible. +/// Assistant media is allowed because the shared fork sanitizer removes it. +/// Provider-specific payload, data-reference, metadata, and size validation +/// remains with the sanitizer and adapters. This does not select candidates. +pub(crate) fn validate_checkout_prefix(transcript: &[Item]) -> Result<(), String> { + let mut seen = HashSet::new(); + let mut pending = HashSet::new(); + for (index, item) in transcript.iter().enumerate() { + if !pending.is_empty() && item.kind != ItemKind::Tool { + return Err(format!( + "checkout prefix item {index} interrupts a pending tool-call batch" + )); + } + if item.kind == ItemKind::Tool && item.parts.is_empty() { + return Err(format!("checkout prefix item {index} has no tool results")); + } + for part in &item.parts { + match part { + Part::ToolCall(call) => { + if item.kind != ItemKind::Assistant { + return Err(format!( + "checkout prefix item {index} has a tool call outside an assistant item" + )); + } + let id = call.id.0.as_str(); + if id.trim().is_empty() { + return Err(format!( + "checkout prefix item {index} has a blank tool-call ID" + )); + } + if !seen.insert(id) { + return Err(format!( + "checkout prefix item {index} reuses a tool-call ID" + )); + } + pending.insert(id); + } + Part::ToolResult(result) => { + if item.kind != ItemKind::Tool { + return Err(format!( + "checkout prefix item {index} has a tool result outside a tool item" + )); + } + let id = result.call_id.0.as_str(); + if id.trim().is_empty() { + return Err(format!( + "checkout prefix item {index} has a blank tool-result ID" + )); + } + if !pending.remove(id) { + return Err(format!( + "checkout prefix item {index} has a duplicate or orphan tool result" + )); + } + } + _ if item.kind == ItemKind::Tool => { + return Err(format!( + "checkout prefix item {index} has non-result content in a tool item" + )); + } + Part::Custom(_) => { + return Err(format!( + "checkout prefix item {index} has unsupported custom content" + )); + } + Part::File(_) if item.kind != ItemKind::User => { + return Err(format!( + "checkout prefix item {index} has file content outside a user item" + )); + } + Part::Media(_) if !matches!(item.kind, ItemKind::User | ItemKind::Assistant) => { + return Err(format!( + "checkout prefix item {index} has media outside a user or assistant item" + )); + } + Part::Media(media) + if item.kind == ItemKind::User + && matches!( + media.modality, + agentkit_core::Modality::Video | agentkit_core::Modality::Binary + ) => + { + return Err(format!( + "checkout prefix item {index} has an unsupported user media modality" + )); + } + _ => {} + } + } + } + if !pending.is_empty() { + return Err("checkout prefix ends with unanswered tool calls".into()); + } + Ok(()) +} + /// Reports whether any tool call in `transcript` is still unanswered. #[must_use] pub fn has_unanswered_tool_calls(transcript: &[Item]) -> bool { @@ -151,6 +255,219 @@ mod tests { ) } + #[test] + fn checkout_accepts_empty_and_closed_parallel_batches_without_changes() { + assert_eq!(validate_checkout_prefix(&[]), Ok(())); + let parallel = Item::new( + ItemKind::Assistant, + vec![call("a").parts.remove(0), call("b").parts.remove(0)], + ); + for results in [ + vec![result("a"), result("b")], + vec![result("b"), result("a")], + vec![Item::new( + ItemKind::Tool, + vec![result("b").parts.remove(0), result("a").parts.remove(0)], + )], + ] { + let mut transcript = vec![Item::text(ItemKind::User, "go"), parallel.clone()]; + transcript.extend(results); + transcript.extend([call("c"), result("c")]); + transcript.push(Item::text(ItemKind::Assistant, "done")); + let before = transcript.clone(); + assert_eq!(validate_checkout_prefix(&transcript), Ok(())); + assert_eq!(transcript, before); + } + } + + #[test] + fn checkout_rejects_invalid_order_roles_ids_and_boundaries() { + let cases = [ + ("unanswered", vec![call("a")]), + ("orphan", vec![result("a")]), + ("before call", vec![result("a"), call("a")]), + ( + "duplicate result", + vec![call("a"), result("a"), result("a")], + ), + ( + "reused ID", + vec![call("a"), result("a"), call("a"), result("a")], + ), + ("blank call", vec![call(" \t"), result(" \t")]), + ("blank result", vec![call("a"), result("\n")]), + ("empty call ID", vec![call(""), result("")]), + ("empty result ID", vec![result("")]), + ( + "nested batch", + vec![call("a"), call("b"), result("b"), result("a")], + ), + ("wrong result ID", vec![call("a"), result("b")]), + ("empty tool item", vec![Item::new(ItemKind::Tool, vec![])]), + ("tool text", vec![Item::text(ItemKind::Tool, "done")]), + ( + "same-item duplicate calls", + vec![ + Item::new( + ItemKind::Assistant, + vec![call("a").parts.remove(0), call("a").parts.remove(0)], + ), + result("a"), + ], + ), + ( + "same-item duplicate results", + vec![ + call("a"), + Item::new( + ItemKind::Tool, + vec![result("a").parts.remove(0), result("a").parts.remove(0)], + ), + ], + ), + ( + "partial parallel batch", + vec![ + Item::new( + ItemKind::Assistant, + vec![call("a").parts.remove(0), call("b").parts.remove(0)], + ), + result("a"), + ], + ), + ( + "mixed tool content", + vec![ + call("a"), + Item::new( + ItemKind::Tool, + vec![result("a").parts.remove(0), Part::text("extra")], + ), + ], + ), + ]; + for (name, transcript) in cases { + assert!(validate_checkout_prefix(&transcript).is_err(), "{name}"); + } + for kind in [ + ItemKind::System, + ItemKind::Developer, + ItemKind::User, + ItemKind::Assistant, + ItemKind::Tool, + ItemKind::Context, + ItemKind::Notification, + ] { + if kind != ItemKind::Assistant { + let mut invalid_call = call("a"); + invalid_call.kind = kind; + assert!(validate_checkout_prefix(&[invalid_call, result("a")]).is_err()); + } + if kind != ItemKind::Tool { + let mut invalid_result = result("a"); + invalid_result.kind = kind; + assert!(validate_checkout_prefix(&[call("a"), invalid_result]).is_err()); + } + // Even an empty intervening item cannot hide an interrupted batch. + assert!( + validate_checkout_prefix(&[call("a"), Item::new(kind, vec![]), result("a"),]) + .is_err() + ); + } + } + + #[test] + fn checkout_preserves_inherited_reasoning_and_media_policy() { + let mut transcript = vec![ + Item::new( + ItemKind::User, + vec![Part::media( + agentkit_core::Modality::Image, + "image/png", + agentkit_core::DataRef::InlineBytes(vec![1]), + )], + ), + Item::new( + ItemKind::Assistant, + vec![ + Part::reasoning("summary"), + Part::media( + agentkit_core::Modality::Image, + "image/png", + agentkit_core::DataRef::InlineBytes(vec![2]), + ), + Part::text("done"), + ], + ), + ]; + let before = transcript.clone(); + assert_eq!(validate_checkout_prefix(&transcript), Ok(())); + assert_eq!(transcript, before); + sanitize_forked_transcript(&mut transcript); + assert_eq!(validate_checkout_prefix(&transcript), Ok(())); + assert_eq!(transcript[0], before[0]); + assert_eq!( + transcript[1].parts, + vec![Part::reasoning("summary"), Part::text("done")] + ); + } + + #[test] + fn checkout_checks_common_role_part_encoding_constraints() { + use agentkit_core::{CustomPart, DataRef, Modality}; + + for kind in [ + ItemKind::System, + ItemKind::Developer, + ItemKind::User, + ItemKind::Assistant, + ItemKind::Tool, + ItemKind::Context, + ItemKind::Notification, + ] { + for (part, supported) in [ + (Part::Custom(CustomPart::new("opaque")), false), + ( + Part::file(DataRef::InlineBytes(vec![1])), + kind == ItemKind::User, + ), + (Part::reasoning("summary"), kind != ItemKind::Tool), + ( + Part::structured(json!({"ok": true})), + kind != ItemKind::Tool, + ), + (Part::text("hello"), kind != ItemKind::Tool), + ( + Part::media(Modality::Image, "image/png", DataRef::InlineBytes(vec![1])), + matches!(kind, ItemKind::User | ItemKind::Assistant), + ), + ( + Part::media(Modality::Audio, "audio/wav", DataRef::InlineBytes(vec![1])), + matches!(kind, ItemKind::User | ItemKind::Assistant), + ), + ( + Part::media(Modality::Video, "video/mp4", DataRef::InlineBytes(vec![1])), + kind == ItemKind::Assistant, // Removed by the shared sanitizer. + ), + ( + Part::media( + Modality::Binary, + "application/octet-stream", + DataRef::InlineBytes(vec![1]), + ), + kind == ItemKind::Assistant, + ), + ] { + let item = Item::new(kind, vec![part]); + assert_eq!( + validate_checkout_prefix(std::slice::from_ref(&item)).is_ok(), + supported, + "{item:?}", + ); + } + } + } + #[test] fn fork_removes_session_bound_continuations_and_generated_images() { let mut continuation = MetadataMap::new(); diff --git a/src/tui/app.rs b/src/tui/app.rs index ecdf50b..46ec8ef 100644 --- a/src/tui/app.rs +++ b/src/tui/app.rs @@ -30,6 +30,7 @@ use unicode_segmentation::UnicodeSegmentation; use crate::compaction::is_compaction_summary; use crate::events::{GenerationOutcome, RuntimeEvent, SubagentStatus}; use crate::file_search::FileMatch; +use crate::protocols::acp::prompt_branches::{PromptBoundary, PromptRole}; const MAX_TOOL_OUTPUT_LINES: usize = 5_000; const MAX_IMAGE_BASE64_BYTES: usize = 14 * 1024 * 1024; @@ -308,12 +309,60 @@ struct SteerEdit { pub(super) const BRANCH_WARNING: &str = "Only conversation context changes. Filesystem changes, running processes, and external effects are not rolled back."; +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub(super) enum BranchFilter { + #[default] + All, + User, + Assistant, + Tool, +} + +impl BranchFilter { + pub fn label(self) -> &'static str { + match self { + Self::All => "All", + Self::User => "User", + Self::Assistant => "Assistant", + Self::Tool => "Tool", + } + } + + fn matches(self, role: &PromptRole) -> bool { + matches!( + (self, role), + (Self::All, _) + | (Self::User, PromptRole::User) + | (Self::Assistant, PromptRole::Assistant) + | (Self::Tool, PromptRole::Tool) + ) + } +} + +pub(super) fn branch_role_label(role: &PromptRole) -> &'static str { + match role { + PromptRole::User => "[user]", + PromptRole::Assistant => "[assistant]", + PromptRole::Tool => "[tool]", + } +} + pub(super) struct BranchChooser { - pub boundaries: Vec, + pub boundaries: Vec, + pub filter: BranchFilter, + /// Index within the filtered list, never a backend address. pub selected: usize, pub pending: bool, } +impl BranchChooser { + pub fn filtered_boundaries(&self) -> impl Iterator { + self.boundaries + .iter() + .filter(|boundary| self.filter.matches(&boundary.role)) + } +} + struct BranchDraft { original: Box, checkout_token: String, @@ -3480,6 +3529,7 @@ impl App { self.branch_epoch = self.branch_epoch.wrapping_add(1); self.branch_chooser = Some(BranchChooser { boundaries: Vec::new(), + filter: BranchFilter::All, selected: 0, pending: true, }); @@ -3502,6 +3552,7 @@ impl App { match result { Ok(boundaries) => { chooser.boundaries = boundaries; + chooser.selected = 0; chooser.pending = false; } Err(error) => { @@ -3625,14 +3676,26 @@ impl App { return Action::None; } match key.code { + KeyCode::Char(c @ ('0'..='3')) if key.modifiers.is_empty() && !pasted => { + chooser.filter = match c { + '1' => BranchFilter::User, + '2' => BranchFilter::Assistant, + '3' => BranchFilter::Tool, + _ => BranchFilter::All, + }; + chooser.selected = 0; + } KeyCode::Up => chooser.selected = chooser.selected.saturating_sub(1), KeyCode::Down => { - chooser.selected = - (chooser.selected + 1).min(chooser.boundaries.len().saturating_sub(1)) + chooser.selected = (chooser.selected + 1) + .min(chooser.filtered_boundaries().count().saturating_sub(1)) } KeyCode::Enter if key.modifiers.is_empty() && !pasted => { - if let Some(boundary) = chooser.boundaries.get(chooser.selected) { - let address = boundary.address.clone(); + let address = chooser + .filtered_boundaries() + .nth(chooser.selected) + .map(|boundary| boundary.address.clone()); + if let Some(address) = address { chooser.pending = true; self.branch_epoch = self.branch_epoch.wrapping_add(1); return Action::PreparePromptBranch { @@ -4489,8 +4552,11 @@ mod tests { }; use super::{ - Action, App, AttachmentKind, Block, MAX_IMAGE_BASE64_BYTES, MAX_IMAGE_SOURCE_BYTES, - MAX_RETAINED_IMAGE_SOURCE_BYTES, Phase, Update, UserImage, + Action, App, AttachmentKind, Block, BranchFilter, MAX_IMAGE_BASE64_BYTES, + MAX_IMAGE_SOURCE_BYTES, MAX_RETAINED_IMAGE_SOURCE_BYTES, Phase, Update, UserImage, + }; + use crate::protocols::acp::prompt_branches::{ + PreparePromptBranchResponse, PromptBoundary, PromptRole, }; use crate::{events::RuntimeEvent, file_search::FileMatch, tui::wrap::LinkHit}; @@ -4615,6 +4681,7 @@ mod tests { epoch, Ok(vec![ crate::protocols::acp::prompt_branches::PromptBoundary { + role: PromptRole::User, address: "opaque".into(), text: "original".into(), historical: false, @@ -4624,6 +4691,180 @@ mod tests { epoch } + #[test] + fn branch_role_filters_select_backend_addresses_not_filtered_positions() { + let mut app = app(); + let epoch = ready_branch_chooser(&mut app); + app.branch_listed( + epoch, + Ok(vec![ + PromptBoundary { + role: PromptRole::User, + address: "user-address".into(), + text: "user".into(), + historical: false, + }, + PromptBoundary { + role: PromptRole::Tool, + address: "tool-address".into(), + text: "tool".into(), + historical: false, + }, + PromptBoundary { + role: PromptRole::Assistant, + address: "assistant-first".into(), + text: "first".into(), + historical: true, + }, + PromptBoundary { + role: PromptRole::Assistant, + address: "assistant-last".into(), + text: "last".into(), + historical: false, + }, + ]), + ); + for (key, filter, count, expected) in [ + ('2', BranchFilter::Assistant, 2, "assistant-last"), + ('3', BranchFilter::Tool, 1, "tool-address"), + ('1', BranchFilter::User, 1, "user-address"), + ('0', BranchFilter::All, 4, "assistant-last"), + ] { + app.handle_branch_key(press(KeyCode::Char(key)), false); + let chooser = app.branch_chooser.as_ref().unwrap(); + assert_eq!(chooser.filter, filter); + assert_eq!(chooser.selected, 0); + assert_eq!(chooser.filtered_boundaries().count(), count); + for _ in 0..5 { + app.handle_branch_key(press(KeyCode::Down), false); + } + assert_eq!(app.branch_chooser.as_ref().unwrap().selected, count - 1); + let Action::PreparePromptBranch { address, epoch } = + app.handle_branch_key(press(KeyCode::Enter), false) + else { + panic!("must prepare selected boundary") + }; + assert_eq!(address, expected); + // A failed prepare keeps the filter and selection available to retry. + app.branch_prepared(epoch, Err("retry".into())); + } + } + + #[test] + fn branch_empty_filter_and_empty_list_are_safe_and_filter_keys_are_unmodified() { + let mut app = app(); + let epoch = ready_branch_chooser(&mut app); + app.handle_branch_key( + modified_press(KeyCode::Char('2'), KeyModifiers::CONTROL), + false, + ); + app.handle_branch_key(press(KeyCode::Char('3')), true); + assert_eq!( + app.branch_chooser.as_ref().unwrap().filter, + BranchFilter::All + ); + app.handle_branch_key(press(KeyCode::Char('2')), false); + for empty_list in [false, true] { + if empty_list { + app.branch_listed(epoch, Ok(Vec::new())); + app.handle_branch_key(press(KeyCode::Char('0')), false); + } + for key in [KeyCode::Down, KeyCode::Up, KeyCode::Enter] { + assert!(matches!( + app.handle_branch_key(press(key), false), + Action::None + )); + } + let chooser = app.branch_chooser.as_ref().unwrap(); + assert_eq!(chooser.filtered_boundaries().count(), 0); + assert_eq!(chooser.selected, 0); + assert!(!chooser.pending); + } + } + + #[test] + fn branch_assistant_and_tool_empty_prefill_require_text_and_restore_source_on_escape() { + for role in [PromptRole::Assistant, PromptRole::Tool] { + let mut app = app(); + app.start_session("source".into()); + app.push_block(Block::Agent("source answer".into())); + app.paste("unsent draft"); + app.attach( + PathBuf::from("/tmp/source.png"), + "image/png", + AttachmentKind::Image, + 1, + ); + let source_text = app.editor.text().to_string(); + let source_attachments = app.attachments.clone(); + let epoch = ready_branch_chooser(&mut app); + app.branch_listed( + epoch, + Ok(vec![PromptBoundary { + role, + address: "backend-address".into(), + text: "preview, not prefill".into(), + historical: false, + }]), + ); + let Action::PreparePromptBranch { epoch, .. } = + app.handle_branch_key(press(KeyCode::Enter), false) + else { + panic!("prepare") + }; + app.branch_prepared( + epoch, + Ok(PreparePromptBranchResponse { + checkout_token: "empty-prefill".into(), + original_text: String::new(), + prefix: Vec::new(), + config_options: Vec::new(), + }), + ); + assert!(app.editing_branch()); + assert!(app.editor.text().is_empty()); + assert!(app.attachments.is_empty()); + for text in ["", " \n\t"] { + app.editor.clear(); + app.editor.insert_str(text); + assert!(matches!( + app.handle_branch_key(press(KeyCode::Enter), false), + Action::None + )); + assert!(!app.branch_submitting()); + } + app.editor.insert_str("continue here"); + let Action::SubmitPromptBranch { + epoch, + text, + checkout_token, + } = app.handle_branch_key(press(KeyCode::Enter), false) + else { + panic!("nonblank continuation must submit") + }; + assert_eq!(text, " \n continue here"); + assert_eq!(checkout_token, "empty-prefill"); + app.branch_submit_failed(epoch, "retry".into()); + app.handle_branch_key(press(KeyCode::Esc), false); + assert_eq!(app.editor.text(), source_text); + assert_eq!(app.attachments, source_attachments); + assert_eq!(app.session_id.as_deref(), Some("source")); + assert!(matches!(&app.blocks[0], Block::Agent(text) if text == "source answer")); + // An empty prepared editor is still guarded against late responses. + app.branch_prepared( + epoch, + Ok(PreparePromptBranchResponse { + checkout_token: "late".into(), + original_text: String::new(), + prefix: Vec::new(), + config_options: Vec::new(), + }), + ); + assert!(!app.editing_branch()); + assert_eq!(app.editor.text(), source_text); + } + } + #[test] fn branch_chooser_and_draft_exclude_slow_processing_but_preserve_real_input_gaps() { // Inject a monotonic clock rather than sleeping: rendering/updating takes @@ -4957,6 +5198,7 @@ mod tests { app.branch_listed( epoch, Ok(vec![PromptBoundary { + role: PromptRole::User, address: "stale".into(), text: "stale".into(), historical: false, diff --git a/src/tui/ui.rs b/src/tui/ui.rs index 72b5b7d..38fe8ef 100644 --- a/src/tui/ui.rs +++ b/src/tui/ui.rs @@ -626,10 +626,11 @@ fn draw_branch_chooser(frame: &mut Frame<'_>, app: &App) { let inner = panel.inner(area); frame.render_widget(Clear, area); frame.render_widget(panel, area); - let [warning, entries, help] = Layout::vertical([ + let [warning, filters, entries, help] = Layout::vertical([ Constraint::Length( (super::app::BRANCH_WARNING.len() as u16 / inner.width.max(1) + 2).min(7), ), + Constraint::Length(1), Constraint::Min(1), Constraint::Length(2), ]) @@ -640,17 +641,25 @@ fn draw_branch_chooser(frame: &mut Frame<'_>, app: &App) { .wrap(ratatui::widgets::Wrap { trim: false }), warning, ); + frame.render_widget( + Paragraph::new(format!( + "0 All · 1 User · 2 Assistant · 3 Tool · Filter: {}", + chooser.filter.label() + )), + filters, + ); let rows = if chooser.pending { vec![Line::from("Loading checkout… Esc cancels")] - } else if chooser.boundaries.is_empty() { - vec![Line::from("No eligible text prompts in this session.")] + } else if chooser.filtered_boundaries().next().is_none() { + vec![Line::from( + "No eligible text checkpoints match this filter.", + )] } else { let start = chooser .selected .saturating_sub(entries.height.saturating_sub(1) as usize); chooser - .boundaries - .iter() + .filtered_boundaries() .enumerate() .skip(start) .take(entries.height as usize) @@ -663,12 +672,13 @@ fn draw_branch_chooser(frame: &mut Frame<'_>, app: &App) { .collect(); Line::styled( format!( - "{} {}{} {}", + "{} {} {}{} {}", if index == chooser.selected { "›" } else { " " }, + super::app::branch_role_label(&boundary.role), if boundary.historical { "[archived] " } else { @@ -687,7 +697,7 @@ fn draw_branch_chooser(frame: &mut Frame<'_>, app: &App) { .collect() }; frame.render_widget(Paragraph::new(rows), entries); - frame.render_widget(Paragraph::new("↑/↓ select · Enter edit in provisional draft · Esc back\nNo branch is created until the edited draft is submitted."), help); + frame.render_widget(Paragraph::new("↑/↓ select · Enter prepare draft · Esc back\nNo branch is created until a nonblank text draft is submitted."), help); } /// A read-only index of display text. Only visible entries build previews. @@ -3165,10 +3175,12 @@ mod tests { app.session_id = Some("source".into()); app.branch_chooser = Some(crate::tui::app::BranchChooser { boundaries: vec![PromptBoundary { + role: crate::protocols::acp::prompt_branches::PromptRole::User, address: "opaque-boundary".into(), text: "editable text".into(), historical: true, }], + filter: crate::tui::app::BranchFilter::All, selected: 0, pending: false, }); @@ -3199,6 +3211,64 @@ mod tests { assert!(output.contains("Esc abandon"), "{output}"); } + #[test] + fn prompt_branch_chooser_renders_role_filters_and_empty_matches() { + use crate::protocols::acp::prompt_branches::{PromptBoundary, PromptRole}; + use crate::tui::app::{BranchChooser, BranchFilter}; + let mut app = navigation_app(Vec::new()); + app.branch_chooser = Some(BranchChooser { + boundaries: vec![ + PromptBoundary { + role: PromptRole::User, + address: "user-address".into(), + text: "user preview".into(), + historical: false, + }, + PromptBoundary { + role: PromptRole::Assistant, + address: "assistant-address".into(), + text: "assistant preview".into(), + historical: true, + }, + PromptBoundary { + role: PromptRole::Tool, + address: "tool-address".into(), + text: "tool preview".into(), + historical: false, + }, + ], + filter: BranchFilter::All, + selected: 0, + pending: false, + }); + let output = render(&mut app, 100, 24); + for text in [ + "[user] user-address", + "[assistant] [archived] assistant-address", + "[tool] tool-address", + "0 All · 1 User · 2 Assistant · 3 Tool", + "Filter: All", + ] { + assert!(output.contains(text), "{output}"); + } + app.branch_chooser.as_mut().unwrap().filter = BranchFilter::Assistant; + let output = render(&mut app, 100, 24); + assert!(output.contains("Filter: Assistant"), "{output}"); + assert!( + output.contains("› [assistant] [archived] assistant-address"), + "{output}" + ); + assert!(!output.contains("user-address"), "{output}"); + assert!(!output.contains("tool-address"), "{output}"); + app.branch_chooser.as_mut().unwrap().boundaries.clear(); + let output = render(&mut app, 100, 24); + assert!( + output.contains("No eligible text checkpoints match this filter."), + "{output}" + ); + assert!(output.contains("Esc back"), "{output}"); + } + #[test] fn navigation_reveal_uses_wrapped_prefix_and_reanchors_on_resize() { let mut app = navigation_app(vec![