From e138fff1ba1d3e40f3d827eaffa3a40e1a4ae614 Mon Sep 17 00:00:00 2001 From: harryalbert Date: Tue, 30 Jun 2026 17:19:26 -0400 Subject: [PATCH 1/6] write tech spec --- specs/tui-agent-tool-calls/TECH.md | 190 +++++++++++++++++++++++++++++ 1 file changed, 190 insertions(+) create mode 100644 specs/tui-agent-tool-calls/TECH.md diff --git a/specs/tui-agent-tool-calls/TECH.md b/specs/tui-agent-tool-calls/TECH.md new file mode 100644 index 0000000000..9d9f3e1e84 --- /dev/null +++ b/specs/tui-agent-tool-calls/TECH.md @@ -0,0 +1,190 @@ +# TUI Agent Tool Calls TECH +## Context +The TUI transcript renders terminal blocks and simple AI exchange blocks in canonical terminal block-list order. AI exchange blocks currently render user input plus streamed plain-text output, but action/tool-call messages are invisible in the transcript even though the shared Agent Mode controller already receives and queues those actions. +Relevant code at `526ade4522df0e65f138c67dcbcb90f1a3ce63e9`: +- [`crates/warp_tui/src/session.rs`](https://github.com/warpdotdev/warp/blob/526ade4522df0e65f138c67dcbcb90f1a3ce63e9/crates/warp_tui/src/session.rs) — boots the headless app, creates `LocalTtyTerminalManager::`, and keeps the TUI driver plus terminal manager alive. +- [`crates/warp_tui/src/terminal_session_view.rs`](https://github.com/warpdotdev/warp/blob/526ade4522df0e65f138c67dcbcb90f1a3ce63e9/crates/warp_tui/src/terminal_session_view.rs) — constructs the production AI stack for the TUI surface: `ActiveSession`, `TuiConversationSelection`, `BlocklistAIContextModel`, `BlocklistAIInputModel`, `BlocklistAIActionModel`, and `BlocklistAIController`. +- [`crates/warp_tui/src/transcript_view.rs`](https://github.com/warpdotdev/warp/blob/526ade4522df0e65f138c67dcbcb90f1a3ce63e9/crates/warp_tui/src/transcript_view.rs) — subscribes to `BlocklistAIHistoryModel`, creates `TuiAgentBlockView`s for visible exchanges, and appends them to `TerminalModel::BlockList` as `RichContentType::AIBlock`. +- [`crates/warp_tui/src/tui_block_list_viewport_source.rs`](https://github.com/warpdotdev/warp/blob/526ade4522df0e65f138c67dcbcb90f1a3ce63e9/crates/warp_tui/src/tui_block_list_viewport_source.rs) — walks the canonical block-list sum tree, measures dirty rich-content views, updates cached heights, and renders terminal blocks plus TUI agent blocks. +- [`crates/warp_tui/src/agent_block.rs`](https://github.com/warpdotdev/warp/blob/526ade4522df0e65f138c67dcbcb90f1a3ce63e9/crates/warp_tui/src/agent_block.rs) — currently derives `AgentBlockContent` from `output.text_from_agent_output()`, which only yields `AIAgentOutputMessageType::Text` and therefore drops action/tool-call messages. +- [`app/src/ai/agent/mod.rs`](https://github.com/warpdotdev/warp/blob/526ade4522df0e65f138c67dcbcb90f1a3ce63e9/app/src/ai/agent/mod.rs) — defines `AIAgentOutput`, ordered `AIAgentOutputMessage`s, `AIAgentOutputMessageType::Action(AIAgentAction)`, and helper iterators such as `text_from_agent_output()` and `actions()`. +- [`app/src/ai/blocklist/block/view_impl/output.rs`](https://github.com/warpdotdev/warp/blob/526ade4522df0e65f138c67dcbcb90f1a3ce63e9/app/src/ai/blocklist/block/view_impl/output.rs) — GUI AI block rendering loops over `output.messages` in order and delegates each message variant to the appropriate renderer. +- [`app/src/ai/blocklist/controller.rs`](https://github.com/warpdotdev/warp/blob/526ade4522df0e65f138c67dcbcb90f1a3ce63e9/app/src/ai/blocklist/controller.rs) — when a response stream finishes, queues emitted actions through `BlocklistAIActionModel::queue_actions` and later sends completed action results back to the model. +- [`app/src/ai/blocklist/action_model.rs`](https://github.com/warpdotdev/warp/blob/526ade4522df0e65f138c67dcbcb90f1a3ce63e9/app/src/ai/blocklist/action_model.rs) and [`app/src/ai/blocklist/action_model/execute.rs`](https://github.com/warpdotdev/warp/blob/526ade4522df0e65f138c67dcbcb90f1a3ce63e9/app/src/ai/blocklist/action_model/execute.rs) — own the shared action queue, preprocessing, permission checks, serial/parallel scheduling, cancellation, and per-action execution dispatch. +- [`app/src/ai/blocklist/action_model/execute/request_file_edits.rs`](https://github.com/warpdotdev/warp/blob/526ade4522df0e65f138c67dcbcb90f1a3ce63e9/app/src/ai/blocklist/action_model/execute/request_file_edits.rs) — currently requires a registered GUI `CodeDiffView` before `RequestFileEdits` can execute. +The current TUI transcript work deliberately uses production-shaped models instead of a separate TUI-only conversation pipeline. This feature should keep that direction: render action messages in the TUI, but continue to execute tool calls through the shared action model. +## Proposed changes +### End-to-end data flow +```mermaid +flowchart LR + User["TUI input submit"] --> Session["TuiTerminalSessionView"] + Session --> Controller["BlocklistAIController"] + Controller --> Stream["AI response stream"] + Stream --> History["BlocklistAIHistoryModel
exchange output"] + Stream --> Actions["BlocklistAIActionModel
queue_actions"] + Actions --> Executor["BlocklistAIActionExecutor
shared tool execution"] + Executor --> Result["AIAgentInput::ActionResult
follow-up request"] + History --> Transcript["TuiTranscriptView
dirty rich content"] + Transcript --> Block["TuiAgentBlockView"] + Block --> Stub["executed a tool call"] +``` +The rendering path and the execution path share the same `AIAgentOutputMessageType::Action` source message but remain separate: +- Rendering derives a transcript item from the stored output message. +- Execution is driven by `BlocklistAIController` and `BlocklistAIActionModel`. +- Completed action results flow back to the LLM as `AIAgentInput::ActionResult`; the first TUI UI does not render those results. +### Ordered message-to-item adapter +Replace `TuiAgentBlockView::content`'s output extraction with an ordered adapter over `AIAgentOutput.messages`. The GUI AI block already follows this shape in `output.rs`: one ordered pass over messages, then variant-specific rendering. The TUI should match the ordering pattern without porting the GUI renderer. +```mermaid +flowchart LR + Output["AIAgentOutput"] --> Messages["messages
server order"] + Messages --> Adapter["TuiAgentBlockView::content"] + Adapter --> Input["Input(String)"] + Adapter --> Text["Message::PlainText(String)"] + Adapter --> Tool["Message::ToolCall(TuiToolCallItem)"] + Tool --> ToolRender["TuiToolCallItem::render"] + ToolRender --> Label["TuiText
executed a tool call"] + Input --> Render["AgentBlockContent::render"] + Text --> Render + Label --> Render +``` +Use a two-level render model so the block stays extensible as more message types get TUI renderers: +```rust +#[derive(Debug, Eq, PartialEq)] +enum TuiAgentBlockItem { + Input(String), + Message(TuiAgentMessageItem), +} + +#[derive(Debug, Eq, PartialEq)] +enum TuiAgentMessageItem { + PlainText(String), + ToolCall(TuiToolCallItem), +} + +#[derive(Debug, Eq, PartialEq)] +struct TuiToolCallItem { + action: AIAgentAction, +} +``` +`TuiToolCallItem` should carry the full `AIAgentAction` even though the first renderer ignores the fields. The item is derived render data, not durable UI state. Carrying the action now gives future renderers direct access to `id`, `task_id`, `requires_result`, and the concrete `AIAgentActionType` without changing the adapter API. +The adapter behavior should be: +- `AIAgentInput::display_query()` values still become `TuiAgentBlockItem::Input`. +- `AIAgentOutputMessageType::Text(AIAgentText { sections })` becomes one `Message(PlainText(...))` per non-empty `AIAgentTextSection::PlainText`. +- `AIAgentOutputMessageType::Action(action)` becomes one `Message(ToolCall(TuiToolCallItem { action: action.clone() }))`. +- Code, table, image, Mermaid, reasoning, summarization, todo, subagent, web, artifact, skill, message-bus, lifecycle-event, debug, and comments-addressed messages remain unsupported until the TUI has specific renderers for them. +`TuiToolCallItem::render` should render exactly `executed a tool call`. The first implementation should not include the tool name, status, arguments, or result details. +### State ownership +Do not store a materialized `Vec` on `TuiAgentBlockView`. Like the GUI AI block, durable state should be stored only when it represents interaction state that must survive renders. +For this first renderer, `TuiAgentBlockView` only needs: +```rust +pub(super) struct TuiAgentBlockView { + model: Rc>, +} +``` +Later stateful renderers can add maps keyed by stable IDs: +- `MessageId` for collapsible reasoning, summarization, web, or todo sections. +- `AIAgentActionId` for expandable or status-aware tool cards. +Even when those maps exist, `output.messages` should remain the ordering source. The render adapter can consult state maps while deriving items, but it should not replace the ordered message pass with independently ordered per-type collections. +### Redraw and height updates +No action-model event subscription is needed for the static tool-call label. New action messages enter the exchange output through the existing response-stream/history path. `TuiTranscriptView::mark_exchange_dirty` already marks the owning rich-content view dirty on `UpdatedStreamingExchange`, and `TuiBlockListViewportSource` already measures dirty rich-content views and writes updated heights back to the terminal block list. +```mermaid +flowchart LR + Event["UpdatedStreamingExchange"] --> Dirty["TuiTranscriptView::mark_exchange_dirty"] + Dirty --> Mark["block_list_mut().mark_rich_content_dirty(view_id)"] + Mark --> Source["TuiBlockListViewportSource::visible_items"] + Source --> Measure["TuiAgentBlockView::desired_height"] + Measure --> Heights["update_rich_content_heights"] + Source --> Render["render child view"] +``` +When future TUI tool cards show live states like queued, blocked, running, failed, or cancelled, `TuiTranscriptView` should subscribe to `BlocklistAIActionModel` events and dirty the owning rich-content item by action ID. That coupling should not be added until visible status depends on it. +### Automatic execution policy +TUI-created conversations should opt into `AIConversationAutoexecuteMode::RunToCompletion` through `TuiConversationSelection`, so emitted tool calls can execute without a first-pass TUI approval UI. +Add an explicit constructor: +```rust +pub(super) fn new_with_autoexecute_override( + terminal_surface_id: EntityId, + autoexecute_override: AIConversationAutoexecuteMode, + ctx: &mut ModelContext>, +) -> Self +``` +Use this constructor from `TuiTerminalSessionView::new` with `AIConversationAutoexecuteMode::RunToCompletion`. Keep `toggle_pending_query_autoexecute` intact so a future TUI affordance can switch between `RunToCompletion` and `RespectUserSettings`. +Do not duplicate permission logic in the TUI. `BlocklistAIPermissions`, execution profiles, and autonomous/sandboxed execution behavior remain inside the shared action execution path. +### Shell-command PTY bridge +Shell-command tool calls require one TUI-specific bridge because the shared shell command executor emits model events rather than directly writing to the TUI PTY. +Add PTY-driving variants to `TuiTerminalSessionEvent`: +```rust +pub(crate) enum TuiTerminalSessionEvent { + ExecuteCommand(Box), + WriteAgentInput { + bytes: Cow<'static, [u8]>, + mode: AIAgentPtyWriteMode, + }, +} +``` +Update `PtyIntentEvent` so: +- `ExecuteCommand` maps to `PtyIntent::ExecuteCommand`. +- `WriteAgentInput` maps to `PtyIntent::WriteAgentInput`. +Subscribe `TuiTerminalSessionView` to `action_model.as_ref(ctx).shell_command_executor(ctx)` and translate: +- `ShellCommandExecutorEvent::ExecuteCommand { action_id, command }` into `TuiTerminalSessionEvent::ExecuteCommand`. +- `ShellCommandExecutorEvent::WriteToPty { input, mode }` into `TuiTerminalSessionEvent::WriteAgentInput`. +- `CancelExecution` and `TransferControlToUser { .. }` as no-ops for the first pass. +The command event should use `CommandExecutionSource::AI` and `AgentInteractionMetadata::new_hidden(action_id, conversation_id)`. The conversation ID should be looked up with `BlocklistAIHistoryModel::conversation_id_for_action(action_id, ctx.view_id())`, and the active session ID should come from the current terminal model active block. +### Shared headless file-edit execution +`RequestFileEdits` currently depends on a GUI `CodeDiffView`: without a registered diff view, `RequestFileEditsExecutor::execute` returns `NotReady`. The TUI should not create a GUI diff view just to make file-edit tool calls executable. +Add a shared, non-GUI fallback inside `RequestFileEditsExecutor`: +- During `preprocess_action`, if diff application succeeds and no `CodeDiffView` is registered, store prepared diffs by `AIAgentActionId`. +- During `execute`, prefer the existing GUI `CodeDiffView` path when a view is registered. +- If no view is registered but prepared diffs exist, save those diffs through the same local/remote file backend semantics used by the GUI path and return `RequestFileEditsResult::Success`. +- If preprocessing failed, return `RequestFileEditsResult::DiffApplicationFailed` so the LLM can recover in a follow-up. +This is not a TUI renderer concern. It belongs in the shared executor so future headless or non-GUI surfaces can execute file edits without depending on GUI views. +### Public TUI export boundary +Extend `app/src/tui_export.rs` only for types that `warp_tui` must name directly: +- `AIAgentAction`, `AIAgentActionId`, `AIAgentActionType`, and `TaskId` for `TuiToolCallItem` and test fixtures. +- `AIAgentPtyWriteMode`, `AgentInteractionMetadata`, and `ShellCommandExecutorEvent` for the PTY bridge. +Do not export action status/result types for this feature. The static stub renderer does not need them. +### Boundaries and non-goals +Do not port GUI inline-action components into the TUI. `RequestedCommand`, `CodeDiffView`, `RunAgentsCardView`, and `AskUserQuestionView` remain GUI-specific. +Do not render full tool results in this change. Finished tool results continue to flow back to the model as `AIAgentInput::ActionResult` through `BlocklistAIController`. +Do not add TUI approval editors for commands, run-agents configs, or ask-user-question answers. `RunToCompletion` is the first-step execution policy. +Do not change canonical transcript ordering. Agent blocks remain `RichContentType::AIBlock` entries in `TerminalModel::BlockList`, and terminal blocks continue to render through `render_terminal_block_rows`. +## Testing and validation +Add unit tests in `crates/warp_tui/src/agent_block_tests.rs` for the ordered message adapter: +- `Text -> Action -> Text` renders as text, `executed a tool call`, text in that order. +- Multiple action messages render multiple `TuiToolCallItem` stub lines. +- `TuiToolCallItem` preserves the action ID and action type in derived content even though rendering only emits the static label. +- Unsupported text sections and unsupported message variants are ignored without disturbing adjacent supported items. +- `desired_height` accounts for the tool-call stub line. +Keep transcript dirty/reflow tests focused on `UpdatedStreamingExchange`; no action-model event dirtying test is needed for the static stub. +Add shell bridge tests: +- `TuiTerminalSessionEvent::ExecuteCommand` maps to `PtyIntent::ExecuteCommand`. +- `TuiTerminalSessionEvent::WriteAgentInput` maps to `PtyIntent::WriteAgentInput`. +- Shell executor event translation emits an AI-sourced `ExecuteCommandEvent` when an active session and owning conversation can be found. +Add or port `RequestFileEditsExecutor` tests for the no-`CodeDiffView` path: +- Prepared diff success saves files and returns `RequestFileEditsResult::Success`. +- Save failure returns `DiffApplicationFailed`. +- Preprocessing failure returns `DiffApplicationFailed`. +Run targeted tests first: +```bash +cargo test -p warp_tui agent_block +cargo test -p warp_tui transcript_view +cargo test -p warp_tui tui_block_list_viewport_source +cargo test -p warp request_file_edits +``` +Then run formatting and the standard Rust validation for touched crates: +```bash +./script/format +cargo clippy --workspace --all-targets --all-features --tests -- -D warnings +``` +## Parallelization +Do not use child agents for the implementation. The work is tightly coupled across one TUI surface, one render adapter, shared executor exports, and one shared executor fallback. Splitting implementation across worktrees would create more merge overhead than wall-clock savings. +The implementation sequence should be: +```mermaid +flowchart LR + A["TUI exports"] --> B["Ordered TUI render adapter"] + B --> C["Autoexecute selection constructor"] + C --> D["Shell PTY bridge"] + D --> E["Headless file-edit fallback"] + E --> F["Targeted tests"] + F --> G["Format + clippy"] +``` +The shell bridge and file-edit fallback can be reviewed as separate conceptual units, but they should land in one branch because the user-facing feature is one TUI tool-call execution path. From 6213e0e8fe284bac76cb264905bda9564ba83c6d Mon Sep 17 00:00:00 2001 From: harryalbert Date: Wed, 1 Jul 2026 10:16:21 -0400 Subject: [PATCH 2/6] tool calling --- .../execute/request_file_edits.rs | 242 ++++++++++++++++-- .../execute/request_file_edits_tests.rs | 95 ++++++- app/src/ai/blocklist/mod.rs | 7 +- app/src/tui_export.rs | 7 +- crates/warp_tui/src/agent_block.rs | 61 ++++- crates/warp_tui/src/agent_block_tests.rs | 110 +++++++- crates/warp_tui/src/conversation_selection.rs | 37 ++- crates/warp_tui/src/terminal_session_view.rs | 116 ++++++++- specs/tui-agent-tool-calls/TECH.md | 68 +++-- 9 files changed, 629 insertions(+), 114 deletions(-) diff --git a/app/src/ai/blocklist/action_model/execute/request_file_edits.rs b/app/src/ai/blocklist/action_model/execute/request_file_edits.rs index e6f6e9a72e..e93b2391f1 100644 --- a/app/src/ai/blocklist/action_model/execute/request_file_edits.rs +++ b/app/src/ai/blocklist/action_model/execute/request_file_edits.rs @@ -3,9 +3,11 @@ mod diff_application; mod telemetry; use std::collections::HashMap; -use std::path::PathBuf; +use std::fs; +use std::ops::Range; +use std::path::{Path, PathBuf}; -use ai::diff_validation::AIRequestedCodeDiff; +use ai::diff_validation::{AIRequestedCodeDiff, DiffDelta, DiffType}; use apply_diff_model::ApplyDiffModel; use diff_application::DiffApplicationError; pub(crate) use diff_application::{apply_edits, FileReadResult}; @@ -13,6 +15,7 @@ use futures::channel::oneshot; use futures::future::BoxFuture; use futures::FutureExt; use itertools::Itertools; +use similar::{DiffOp, TextDiff}; pub(crate) use telemetry::MalformedFinalLineProxyEvent; #[allow(unused_imports)] pub use telemetry::{EditAcceptAndContinueClickedEvent, EditAcceptClickedEvent}; @@ -37,6 +40,7 @@ use crate::ai::blocklist::inline_action::code_diff_view::{ }; use crate::ai::blocklist::{BlocklistAIPermissions, RequestedEditResolution}; use crate::ai::paths::host_native_absolute_path; +use crate::code::DiffResult; use crate::terminal::model::session::active_session::ActiveSession; use crate::terminal::model::session::SessionType; use crate::{safe_warn, BlocklistAIHistoryModel}; @@ -46,6 +50,8 @@ pub struct RequestFileEditsExecutor { active_session: ModelHandle, apply_diff_model: ModelHandle, diff_views: HashMap>, + /// Prepared diffs keyed by action ID, used by non-GUI run-to-completion surfaces. + prepared_diffs: HashMap>, /// Set of action IDs where diff application failed. diff_application_failures: HashMap>, terminal_view_id: EntityId, @@ -62,6 +68,7 @@ impl RequestFileEditsExecutor { active_session, apply_diff_model, diff_views: HashMap::new(), + prepared_diffs: HashMap::new(), diff_application_failures: HashMap::new(), terminal_view_id, } @@ -146,11 +153,6 @@ impl RequestFileEditsExecutor { return ActionExecution::InvalidAction; }; - let Some(diff_view) = self.diff_views.get(id) else { - log::warn!("Tried to execute a RequestFileEdits action without a diff view"); - return ActionExecution::NotReady; - }; - // If diff application failed, early exit. if let Some(errors) = self.diff_application_failures.remove(id) { return ActionExecution::Sync(AIAgentActionResultType::RequestFileEdits( @@ -160,6 +162,16 @@ impl RequestFileEditsExecutor { )); } + let Some(diff_view) = self.diff_views.get(id) else { + let Some(prepared_diffs) = self.prepared_diffs.remove(id) else { + log::warn!("Tried to execute a RequestFileEdits action without prepared diffs"); + return ActionExecution::NotReady; + }; + return ActionExecution::Sync(AIAgentActionResultType::RequestFileEdits( + save_prepared_diffs(prepared_diffs), + )); + }; + let identifiers = self .generate_ai_identifiers(&input.conversation_id, id, ctx) .unwrap_or_else(|| AIIdentifiers { @@ -317,13 +329,6 @@ impl RequestFileEditsExecutor { ) { tx.send(()).ok(); - let Some(diff_view) = self.diff_views.get(&id) else { - log::warn!( - "Tried to apply diffs for a RequestFileEdits action without a corresponding diff view" - ); - return; - }; - let applied_diffs = match applied_diffs { Ok(diffs) if !diffs.is_empty() => diffs, Ok(_) => { @@ -362,19 +367,23 @@ impl RequestFileEditsExecutor { diffs.push(file_diff); } - // Set the session type on the diff view so save/delete/create routes - // through the correct FileModel backend. - let diff_session_type = match self.active_session.as_ref(ctx).session_type(ctx) { - Some(SessionType::WarpifiedRemote { - host_id: Some(host_id), - }) => DiffSessionType::Remote(host_id.clone()), - _ => DiffSessionType::Local, - }; - - diff_view.update(ctx, |diff_view, ctx| { - diff_view.set_diff_session_type(diff_session_type); - diff_view.set_candidate_diffs(diffs, ctx); - }); + self.prepared_diffs.insert(id.clone(), diffs.clone()); + + if let Some(diff_view) = self.diff_views.get(&id) { + // Set the session type on the diff view so save/delete/create routes + // through the correct FileModel backend. + let diff_session_type = match self.active_session.as_ref(ctx).session_type(ctx) { + Some(SessionType::WarpifiedRemote { + host_id: Some(host_id), + }) => DiffSessionType::Remote(host_id.clone()), + _ => DiffSessionType::Local, + }; + + diff_view.update(ctx, |diff_view, ctx| { + diff_view.set_diff_session_type(diff_session_type); + diff_view.set_candidate_diffs(diffs, ctx); + }); + } } fn generate_ai_identifiers( @@ -470,6 +479,185 @@ fn updated_file_contexts_from_editor_buffers( .collect() } +/// Saves prepared diffs without requiring a GUI review view. +fn save_prepared_diffs(prepared_diffs: Vec) -> RequestFileEditsResult { + let mut combined_diff = DiffResult::default(); + let mut updated_files = Vec::new(); + let mut deleted_files = Vec::new(); + let mut content_map = HashMap::new(); + + for diff in prepared_diffs { + match save_prepared_diff(diff) { + Ok(saved) => { + combined_diff += &saved.diff; + if let Some(content) = saved.content { + content_map.insert(saved.file_location.name.clone(), content); + updated_files.push((saved.file_location, false)); + } + deleted_files.extend(saved.deleted_files); + } + Err(error) => { + return RequestFileEditsResult::DiffApplicationFailed { error }; + } + } + } + + RequestFileEditsResult::Success { + diff: combined_diff.unified_diff, + updated_files: updated_file_contexts_from_editor_buffers(&updated_files, &content_map), + deleted_files, + lines_added: combined_diff.lines_added, + lines_removed: combined_diff.lines_removed, + } +} + +struct SavedPreparedDiff { + diff: DiffResult, + file_location: FileLocations, + content: Option, + deleted_files: Vec, +} + +fn save_prepared_diff(diff: FileDiff) -> Result { + let path = diff.file_path(); + let mut deleted_files = Vec::new(); + let (target_path, next_content) = match &diff.diff_type { + DiffType::Create { delta } => (path.clone(), delta.insertion.clone()), + DiffType::Update { deltas, rename } => { + let target_path = rename + .as_ref() + .map(|rename| rename.to_string_lossy().to_string()) + .unwrap_or_else(|| path.clone()); + let next_content = apply_deltas_to_content(&diff.base.content, deltas)?; + if target_path != path { + deleted_files.push(path.clone()); + } + (target_path, next_content) + } + DiffType::Delete { .. } => { + fs::remove_file(&path).map_err(|error| format!("Failed to delete {path}: {error}"))?; + return Ok(SavedPreparedDiff { + diff: diff_result(&diff.base.content, "", &path), + file_location: FileLocations { + name: path.clone(), + lines: vec![], + }, + content: None, + deleted_files: vec![path], + }); + } + }; + + if let Some(parent) = Path::new(&target_path).parent() { + fs::create_dir_all(parent) + .map_err(|error| format!("Failed to create parent directory {parent:?}: {error}"))?; + } + fs::write(&target_path, &next_content) + .map_err(|error| format!("Failed to write {target_path}: {error}"))?; + if target_path != path { + fs::remove_file(&path).map_err(|error| format!("Failed to remove {path}: {error}"))?; + } + + Ok(SavedPreparedDiff { + diff: diff_result(&diff.base.content, &next_content, &target_path), + file_location: FileLocations { + name: target_path, + lines: changed_lines_for_headless_result(&diff.diff_type), + }, + content: Some(next_content), + deleted_files, + }) +} + +fn apply_deltas_to_content(content: &str, deltas: &[DiffDelta]) -> Result { + let mut lines = split_lines_preserving_newlines(content); + let mut deltas = deltas.to_vec(); + deltas.sort_by_key(|delta| delta.replacement_line_range.start); + + for delta in deltas.into_iter().rev() { + let start = delta.replacement_line_range.start.saturating_sub(1); + let end = delta.replacement_line_range.end.saturating_sub(1); + if start > lines.len() || end > lines.len() || start > end { + return Err(format!( + "Diff range {:?} is out of bounds for file with {} lines", + delta.replacement_line_range, + lines.len() + )); + } + let replacement = split_lines_preserving_newlines(&delta.insertion); + lines.splice(start..end, replacement); + } + + Ok(lines.concat()) +} + +fn split_lines_preserving_newlines(content: &str) -> Vec { + if content.is_empty() { + Vec::new() + } else { + content.split_inclusive('\n').map(str::to_string).collect() + } +} + +fn diff_result(before: &str, after: &str, file_name: &str) -> DiffResult { + if before == after { + return DiffResult::default(); + } + + let text_diff = TextDiff::from_lines(before, after); + let mut lines_added = 0; + let mut lines_removed = 0; + for op in text_diff.ops() { + match op { + DiffOp::Equal { .. } => {} + DiffOp::Delete { old_len, .. } => lines_removed += old_len, + DiffOp::Insert { new_len, .. } => lines_added += new_len, + DiffOp::Replace { + old_len, new_len, .. + } => { + lines_removed += old_len; + lines_added += new_len; + } + } + } + + DiffResult { + unified_diff: text_diff + .unified_diff() + .context_radius(3) + .header(file_name, file_name) + .missing_newline_hint(false) + .to_string(), + lines_added, + lines_removed, + } +} + +fn changed_lines_for_headless_result(diff_type: &DiffType) -> Vec> { + match diff_type { + DiffType::Create { delta } => inserted_content_range(1, &delta.insertion) + .into_iter() + .collect(), + DiffType::Update { deltas, .. } => deltas + .iter() + .filter_map(changed_line_range_for_delta) + .collect(), + DiffType::Delete { .. } => vec![], + } +} + +fn changed_line_range_for_delta(delta: &DiffDelta) -> Option> { + let replacement_range = &delta.replacement_line_range; + if replacement_range.start == replacement_range.end { + return inserted_content_range(replacement_range.start.max(1), &delta.insertion); + } + Some(replacement_range.clone()) +} + +fn inserted_content_range(start: usize, content: &str) -> Option> { + let line_count = content.lines().count(); + (line_count > 0).then_some(start..start + line_count) +} fn clamp_to_file_context_range_start(file_location: &mut FileLocations) { for range in &mut file_location.lines { range.start = range.start.max(1); diff --git a/app/src/ai/blocklist/action_model/execute/request_file_edits_tests.rs b/app/src/ai/blocklist/action_model/execute/request_file_edits_tests.rs index 3ea2a50eaf..0bfad07b8b 100644 --- a/app/src/ai/blocklist/action_model/execute/request_file_edits_tests.rs +++ b/app/src/ai/blocklist/action_model/execute/request_file_edits_tests.rs @@ -1,9 +1,12 @@ use std::collections::HashMap; +use std::fs; -use ai::agent::action_result::AnyFileContent; +use ai::agent::action_result::{AnyFileContent, RequestFileEditsResult}; use ai::agent::FileLocations; +use ai::diff_validation::{DiffDelta, DiffType}; -use super::updated_file_contexts_from_editor_buffers; +use super::{save_prepared_diffs, updated_file_contexts_from_editor_buffers}; +use crate::ai::blocklist::inline_action::code_diff_view::FileDiff; #[test] fn updated_file_contexts_from_editor_buffers_returns_changed_lines_with_context() { @@ -38,6 +41,94 @@ fn updated_file_contexts_from_editor_buffers_returns_changed_lines_with_context( ); } +#[test] +fn save_prepared_diffs_creates_file_without_code_diff_view() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("src").join("main.rs"); + let path = path.to_string_lossy().to_string(); + + let result = save_prepared_diffs(vec![FileDiff::new( + String::new(), + path.clone(), + DiffType::creation("fn main() {}\n".to_owned()), + )]); + + let RequestFileEditsResult::Success { + updated_files, + deleted_files, + lines_added, + lines_removed, + .. + } = result + else { + panic!("expected create diff to save successfully"); + }; + + assert_eq!(fs::read_to_string(&path).unwrap(), "fn main() {}\n"); + assert_eq!(deleted_files, Vec::::new()); + assert_eq!(lines_added, 1); + assert_eq!(lines_removed, 0); + assert_eq!(updated_files.len(), 1); + assert_eq!(updated_files[0].file_context.file_name, path); +} + +#[test] +fn save_prepared_diffs_updates_file_without_code_diff_view() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("main.rs"); + let path = path.to_string_lossy().to_string(); + let original = "one\ntwo\nthree\n"; + fs::write(&path, original).unwrap(); + + let result = save_prepared_diffs(vec![FileDiff::new( + original.to_owned(), + path.clone(), + DiffType::update( + vec![DiffDelta { + replacement_line_range: 2..3, + insertion: "TWO\n".to_owned(), + }], + None, + ), + )]); + + let RequestFileEditsResult::Success { + updated_files, + lines_added, + lines_removed, + .. + } = result + else { + panic!("expected update diff to save successfully"); + }; + + assert_eq!(fs::read_to_string(&path).unwrap(), "one\nTWO\nthree\n"); + assert_eq!(lines_added, 1); + assert_eq!(lines_removed, 1); + assert_eq!(updated_files.len(), 1); + assert_eq!(updated_files[0].file_context.file_name, path); +} + +#[test] +fn save_prepared_diffs_reports_save_failure_without_code_diff_view() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().to_string_lossy().to_string(); + + let result = save_prepared_diffs(vec![FileDiff::new( + String::new(), + path, + DiffType::creation("not a directory write\n".to_owned()), + )]); + + let RequestFileEditsResult::DiffApplicationFailed { error } = result else { + panic!("expected save failure"); + }; + assert!( + error.contains("Failed to write"), + "unexpected error message: {error}" + ); +} + #[test] fn updated_file_contexts_from_editor_buffers_preserves_full_file_when_no_ranges() { let updated_files = vec![( diff --git a/app/src/ai/blocklist/mod.rs b/app/src/ai/blocklist/mod.rs index 92e9d81a3a..099ef4e8aa 100644 --- a/app/src/ai/blocklist/mod.rs +++ b/app/src/ai/blocklist/mod.rs @@ -32,14 +32,13 @@ pub(crate) mod codebase_index_speedbump_banner; pub(crate) mod telemetry_banner; pub(super) mod view_util; -pub use action_model::BlocklistAIActionModel; #[cfg_attr(target_family = "wasm", allow(unused_imports))] pub(crate) use action_model::{ apply_edits, read_local_file_context, BlocklistAIActionEvent, FileReadResult, - ReadFileContextResult, RequestFileEditsFormatKind, ShellCommandExecutor, - ShellCommandExecutorEvent, StartAgentExecutor, StartAgentExecutorEvent, StartAgentRequest, - StartAgentRequestId, + ReadFileContextResult, RequestFileEditsFormatKind, StartAgentExecutor, StartAgentExecutorEvent, + StartAgentRequest, StartAgentRequestId, }; +pub use action_model::{BlocklistAIActionModel, ShellCommandExecutor, ShellCommandExecutorEvent}; #[cfg(any(test, feature = "integration_tests"))] pub(crate) use block::model::testing::FakeAIBlockModel; pub(crate) use block::{init, model, AIBlock, AIBlockEvent, RequestedEditResolution}; diff --git a/app/src/tui_export.rs b/app/src/tui_export.rs index c800c1e32c..7bba9637b7 100644 --- a/app/src/tui_export.rs +++ b/app/src/tui_export.rs @@ -4,8 +4,10 @@ pub use crate::ai::agent::api::ServerConversationToken; pub use crate::ai::agent::conversation::{ AIConversationAutoexecuteMode, AIConversationId, ConversationStatus, }; +pub use crate::ai::agent::task::TaskId; pub use crate::ai::agent::{ - AIAgentExchangeId, AIAgentInput, AIAgentOutput, AIAgentOutputMessage, AIAgentOutputMessageType, + AIAgentAction, AIAgentActionId, AIAgentActionType, AIAgentExchangeId, AIAgentInput, + AIAgentOutput, AIAgentOutputMessage, AIAgentOutputMessageType, AIAgentPtyWriteMode, AIAgentText, AIAgentTextSection, MessageId, ServerOutputId, Shared, UserQueryMode, }; pub use crate::ai::blocklist::agent_view::{ @@ -24,6 +26,7 @@ pub use crate::ai::blocklist::history_model::{ }; pub use crate::ai::blocklist::{ BlocklistAIActionModel, BlocklistAIContextModel, BlocklistAIController, BlocklistAIInputModel, + ShellCommandExecutor, ShellCommandExecutorEvent, }; pub use crate::ai::get_relevant_files::controller::GetRelevantFilesController; pub use crate::ai::llms::LLMId; @@ -36,7 +39,7 @@ pub use crate::terminal::local_tty::{ TerminalManager as LocalTtyTerminalManager, TerminalManagerInit, TerminalSurfaceInit, TerminalSurfaceResult, }; -pub use crate::terminal::model::block::{Block, BlockId}; +pub use crate::terminal::model::block::{AgentInteractionMetadata, Block, BlockId}; pub use crate::terminal::model::blockgrid::BlockGrid; pub use crate::terminal::model::blocks::{ BlockHeight, BlockHeightItem, BlockHeightSummary, BlockList, RichContentItem, TotalIndex, diff --git a/crates/warp_tui/src/agent_block.rs b/crates/warp_tui/src/agent_block.rs index 45723ccdfc..4e242fd80c 100644 --- a/crates/warp_tui/src/agent_block.rs +++ b/crates/warp_tui/src/agent_block.rs @@ -3,7 +3,8 @@ use std::rc::Rc; use warp::tui_export::{ - AIAgentExchangeId, AIAgentTextSection, AIBlockModel, AIConversationId, Appearance, + AIAgentAction, AIAgentExchangeId, AIAgentOutputMessageType, AIAgentTextSection, AIBlockModel, + AIConversationId, Appearance, }; use warp_core::ui::color::blend::Blend; // `ThemeFill` is the theme-layer color (it supports blend/opacity); `Fill` below @@ -19,11 +20,13 @@ use warpui_core::{AppContext, Entity, EntityIdMap, TuiView}; const INPUT_PREFIX: &str = "≫ "; -/// Renderable pieces of an agent block; this will grow as we add tool calls and other sub-elements. +/// Renderable pieces of an agent block; this will grow as we render richer sections. #[derive(Clone, Debug, Eq, PartialEq)] enum TuiAIBlockSection { Input(String), PlainText(String), + /// A lightweight status row standing in for an agent tool call. + ToolCall(Box), } /// A thin TUI rich-content view adapter backed by one agent exchange. @@ -103,19 +106,39 @@ impl TuiAIBlock { sections.push(TuiAIBlockSection::Input(input)); } + // Walk output messages in order so tool-call rows interleave with text. if let Some(output) = self.model.status(app).output_to_render() { let output = output.get(); - sections.extend(output.text_from_agent_output().flat_map(|text| { - text.sections.iter().filter_map(|section| match section { - AIAgentTextSection::PlainText { text } => (!text.text().is_empty()) - .then(|| TuiAIBlockSection::PlainText(text.text().to_owned())), - // Add item variants here as the TUI learns to render richer sections. - AIAgentTextSection::Code { .. } - | AIAgentTextSection::Table { .. } - | AIAgentTextSection::Image { .. } - | AIAgentTextSection::MermaidDiagram { .. } => None, - }) - })); + for message in &output.messages { + match &message.message { + AIAgentOutputMessageType::Text(text) => { + sections.extend(text.sections.iter().filter_map(|section| match section { + AIAgentTextSection::PlainText { text } => (!text.text().is_empty()) + .then(|| TuiAIBlockSection::PlainText(text.text().to_owned())), + // Add item variants here as the TUI learns to render richer sections. + AIAgentTextSection::Code { .. } + | AIAgentTextSection::Table { .. } + | AIAgentTextSection::Image { .. } + | AIAgentTextSection::MermaidDiagram { .. } => None, + })); + } + AIAgentOutputMessageType::Action(action) => { + sections.push(TuiAIBlockSection::ToolCall(Box::new(action.clone()))); + } + AIAgentOutputMessageType::Reasoning { .. } + | AIAgentOutputMessageType::Summarization { .. } + | AIAgentOutputMessageType::Subagent(_) + | AIAgentOutputMessageType::TodoOperation(_) + | AIAgentOutputMessageType::WebSearch(_) + | AIAgentOutputMessageType::WebFetch(_) + | AIAgentOutputMessageType::CommentsAddressed { .. } + | AIAgentOutputMessageType::DebugOutput { .. } + | AIAgentOutputMessageType::ArtifactCreated(_) + | AIAgentOutputMessageType::SkillInvoked(_) + | AIAgentOutputMessageType::MessagesReceivedFromAgents { .. } + | AIAgentOutputMessageType::EventsFromAgents { .. } => {} + } + } } sections @@ -191,6 +214,18 @@ impl TuiAIBlockSection { .with_padding_top(top_padding) .finish() } + Self::ToolCall(_action) => { + let text_color = + Fill::from(ThemeFill::from(theme.terminal_colors().bright.black)).into(); + Box::new( + TuiContainer::new( + TuiText::new("executed a tool call").with_style( + TuiStyle::default().fg(text_color).add_modifier(Modifier::DIM), + ), + ) + .with_padding_top(top_padding), + ) + } } } } diff --git a/crates/warp_tui/src/agent_block_tests.rs b/crates/warp_tui/src/agent_block_tests.rs index 14604989f5..b88fb18899 100644 --- a/crates/warp_tui/src/agent_block_tests.rs +++ b/crates/warp_tui/src/agent_block_tests.rs @@ -1,10 +1,10 @@ use std::rc::Rc; use warp::tui_export::{ - AIAgentExchangeId, AIAgentInput, AIAgentOutput, AIAgentOutputMessage, AIAgentOutputMessageType, - AIAgentText, AIAgentTextSection, AIBlockModel, AIBlockOutputStatus, AIConversationId, - AIRequestType, Appearance, LLMId, MessageId, OutputStatusUpdateCallback, ServerOutputId, - Shared, UserQueryMode, + AIAgentAction, AIAgentActionId, AIAgentActionType, AIAgentExchangeId, AIAgentInput, + AIAgentOutput, AIAgentOutputMessage, AIAgentOutputMessageType, AIAgentText, AIAgentTextSection, + AIBlockModel, AIBlockOutputStatus, AIConversationId, AIRequestType, Appearance, LLMId, + MessageId, OutputStatusUpdateCallback, ServerOutputId, Shared, TaskId, UserQueryMode, }; use warp_core::ui::color::blend::Blend; use warp_core::ui::theme::Fill as ThemeFill; @@ -99,6 +99,11 @@ fn expected_output_text_color(app: &AppContext) -> Color { CoreFill::from(ThemeFill::from(theme.terminal_colors().normal.white)).into() } +fn expected_tool_call_text_color(app: &AppContext) -> Color { + let theme = Appearance::as_ref(app).theme(); + CoreFill::from(ThemeFill::from(theme.terminal_colors().bright.black)).into() +} + #[test] fn agent_block_extracts_input_and_plain_text_from_model() { App::test((), |app| async move { @@ -126,6 +131,64 @@ fn agent_block_extracts_input_and_plain_text_from_model() { }); } +#[test] +fn agent_block_renders_tool_calls_in_message_order() { + App::test((), |app| async move { + app.add_singleton_model(|_| Appearance::mock()); + app.read(|app_ctx| { + let action = test_action("action-1"); + let block = test_agent_block(FakeAgentBlockModel { + inputs: Vec::new(), + status: complete_output_messages(vec![ + text_message( + "message-1", + vec![AIAgentTextSection::PlainText { + text: "before".to_owned().into(), + }], + ), + action_message("message-2", action.clone()), + text_message( + "message-3", + vec![AIAgentTextSection::PlainText { + text: "after".to_owned().into(), + }], + ), + ]), + }); + + assert_eq!( + block.sections(app_ctx), + vec![ + TuiAIBlockSection::PlainText("before".to_owned()), + TuiAIBlockSection::ToolCall(Box::new(action.clone())), + TuiAIBlockSection::PlainText("after".to_owned()), + ] + ); + + let mut presenter = TuiPresenter::new(); + let frame = presenter.present_element( + block.render_element(app_ctx), + TuiRect::new(0, 0, 40, 4), + app_ctx, + ); + assert_eq!( + frame + .buffer + .to_lines() + .into_iter() + .map(|line| line.trim_end().to_owned()) + .collect::>(), + vec!["before", "executed a tool call", "after", ""], + ); + assert_eq!( + frame.buffer[(0, 1)].fg, + expected_tool_call_text_color(app_ctx) + ); + assert!(frame.buffer[(0, 1)].modifier.contains(Modifier::DIM)); + }); + }); +} + #[test] fn agent_block_omits_unsupported_sections_until_the_tui_can_render_them() { App::test((), |app| async move { @@ -207,18 +270,47 @@ impl AIBlockModel for FakeAgentBlockModel { /// Builds a completed output status with one text message. fn complete_output(sections: Vec) -> AIBlockOutputStatus { + complete_output_messages(vec![text_message("message-1", sections)]) +} + +/// Builds a completed output status from explicit output messages. +fn complete_output_messages(messages: Vec) -> AIBlockOutputStatus { AIBlockOutputStatus::Complete { output: Shared::new(AIAgentOutput { - messages: vec![AIAgentOutputMessage { - id: MessageId::new("message-1".to_owned()), - message: AIAgentOutputMessageType::Text(AIAgentText { sections }), - citations: Vec::new(), - }], + messages, ..Default::default() }), } } +/// Builds a text output message from plain-text sections. +fn text_message(id: &str, sections: Vec) -> AIAgentOutputMessage { + AIAgentOutputMessage { + id: MessageId::new(id.to_owned()), + message: AIAgentOutputMessageType::Text(AIAgentText { sections }), + citations: Vec::new(), + } +} + +/// Builds an action (tool call) output message. +fn action_message(id: &str, action: AIAgentAction) -> AIAgentOutputMessage { + AIAgentOutputMessage { + id: MessageId::new(id.to_owned()), + message: AIAgentOutputMessageType::Action(action), + citations: Vec::new(), + } +} + +/// Builds a tool-call action for message-ordering tests. +fn test_action(id: &str) -> AIAgentAction { + AIAgentAction { + id: AIAgentActionId::from(id.to_owned()), + task_id: TaskId::new("task-1".to_owned()), + action: AIAgentActionType::InitProject, + requires_result: true, + } +} + /// Builds one user-query input for model-backed extraction tests. fn query_input(query: &str) -> AIAgentInput { AIAgentInput::UserQuery { diff --git a/crates/warp_tui/src/conversation_selection.rs b/crates/warp_tui/src/conversation_selection.rs index 3bb6e8fca5..5d7dbe7ed0 100644 --- a/crates/warp_tui/src/conversation_selection.rs +++ b/crates/warp_tui/src/conversation_selection.rs @@ -8,32 +8,51 @@ use warpui::{AppContext, EntityId, ModelContext, SingletonEntity}; /// TUI-owned next-prompt conversation selection. pub(super) struct TuiConversationSelection { terminal_surface_id: EntityId, + default_autoexecute_override: AIConversationAutoexecuteMode, pending_query_state: PendingQueryState, } impl TuiConversationSelection { /// Creates TUI conversation selection for a terminal surface. + #[cfg(test)] pub(super) fn new( terminal_surface_id: EntityId, ctx: &mut ModelContext>, + ) -> Self { + let default_autoexecute_override = + if warp_core::execution_mode::AppExecutionMode::as_ref(ctx).is_sandboxed() { + AIConversationAutoexecuteMode::RunToCompletion + } else { + AIConversationAutoexecuteMode::RespectUserSettings + }; + Self::new_with_autoexecute_override(terminal_surface_id, default_autoexecute_override, ctx) + } + + /// Creates TUI conversation selection with an explicit new-conversation autoexecute mode. + pub(super) fn new_with_autoexecute_override( + terminal_surface_id: EntityId, + autoexecute_override: AIConversationAutoexecuteMode, + ctx: &mut ModelContext>, ) -> Self { ctx.subscribe_to_model( &BlocklistAIHistoryModel::handle(ctx), |selection, _, event, ctx| selection.handle_history_event(event, ctx), ); - let pending_query_state = - if warp_core::execution_mode::AppExecutionMode::as_ref(ctx).is_sandboxed() { - PendingQueryState::New { - autoexecute_override: AIConversationAutoexecuteMode::RunToCompletion, - } - } else { - PendingQueryState::default() - }; + let pending_query_state = PendingQueryState::New { + autoexecute_override, + }; Self { terminal_surface_id, + default_autoexecute_override: autoexecute_override, pending_query_state, } } + /// Returns the configured new-conversation state for this TUI surface. + fn default_new_conversation_state(&self) -> PendingQueryState { + PendingQueryState::New { + autoexecute_override: self.default_autoexecute_override, + } + } /// Returns the selected existing conversation ID. fn selected_id(&self) -> Option { @@ -120,7 +139,7 @@ impl ConversationSelection for TuiConversationSelection { ctx: &mut ModelContext>, ) { let previous_conversation_id = self.selected_id(); - self.set_pending_query_state(PendingQueryState::default(), ctx); + self.set_pending_query_state(self.default_new_conversation_state(), ctx); if let Some(previous_conversation_id) = previous_conversation_id { Self::emit_deactivated(previous_conversation_id, false, ctx); } diff --git a/crates/warp_tui/src/terminal_session_view.rs b/crates/warp_tui/src/terminal_session_view.rs index cc31458ede..1e93b2e70f 100644 --- a/crates/warp_tui/src/terminal_session_view.rs +++ b/crates/warp_tui/src/terminal_session_view.rs @@ -1,10 +1,13 @@ //! Authenticated terminal-session TUI surface. +use std::borrow::Cow; use warp::editor::CodeEditorModel; use warp::tui_export::{ - ActiveSession, AgentViewEntryOrigin, Appearance, BlocklistAIActionModel, - BlocklistAIContextModel, BlocklistAIController, BlocklistAIInputModel, ConversationSelection, - ConversationSelectionHandle, GetRelevantFilesController, ModelEvent, PtyIntent, PtyIntentEvent, + AIAgentPtyWriteMode, AIConversationAutoexecuteMode, ActiveSession, AgentInteractionMetadata, + AgentViewEntryOrigin, Appearance, BlocklistAIActionModel, BlocklistAIContextModel, + BlocklistAIController, BlocklistAIHistoryModel, BlocklistAIInputModel, CommandExecutionSource, + ConversationSelection, ConversationSelectionHandle, ExecuteCommandEvent, + GetRelevantFilesController, ModelEvent, PtyIntent, PtyIntentEvent, ShellCommandExecutorEvent, TerminalSurface, TerminalSurfaceInit, }; use warp_core::ui::theme::Fill as ThemeFill; @@ -25,12 +28,24 @@ use crate::transcript_view::TuiTranscriptView; const INITIAL_INPUT_WIDTH: u16 = 80; const MAX_INPUT_TEXT_ROWS: u16 = 6; -/// This TUI surface does not emit direct PTY intents. -pub(crate) struct TuiTerminalSessionEvent; +/// Events emitted by the TUI terminal session surface. +pub(crate) enum TuiTerminalSessionEvent { + ExecuteCommand(Box), + WriteAgentInput { + bytes: Cow<'static, [u8]>, + mode: AIAgentPtyWriteMode, + }, +} impl PtyIntentEvent for TuiTerminalSessionEvent { fn pty_intent(&self) -> Option { - None + match self { + Self::ExecuteCommand(event) => Some(PtyIntent::ExecuteCommand((**event).clone())), + Self::WriteAgentInput { bytes, mode } => Some(PtyIntent::WriteAgentInput { + bytes: bytes.clone(), + mode: *mode, + }), + } } } @@ -56,9 +71,13 @@ impl TuiTerminalSessionView { let terminal_surface_id: EntityId = ctx.view_id(); let active_session = ctx.add_model(|ctx| ActiveSession::new(sessions.clone(), model_events.clone(), ctx)); + // Agent-requested commands run to completion so tool calls execute end to end. let conversation_selection = ctx.add_model(|ctx| { - Box::new(TuiConversationSelection::new(terminal_surface_id, ctx)) - as Box + Box::new(TuiConversationSelection::new_with_autoexecute_override( + terminal_surface_id, + AIConversationAutoexecuteMode::RunToCompletion, + ctx, + )) as Box }); let context_model = ctx.add_model(|ctx| { BlocklistAIContextModel::new( @@ -95,7 +114,7 @@ impl TuiTerminalSessionView { ai_input_model, context_model, conversation_selection.clone(), - action_model, + action_model.clone(), active_session, model.clone(), terminal_surface_id, @@ -119,6 +138,13 @@ impl TuiTerminalSessionView { } }); + // Bridge shared shell-tool executor events into terminal-manager PTY intents. + let shell_command_executor = action_model.as_ref(ctx).shell_command_executor(ctx); + let model_for_shell_events = model.clone(); + ctx.subscribe_to_model(&shell_command_executor, move |view, _, event, ctx| { + view.handle_shell_command_executor_event(event, &model_for_shell_events, ctx); + }); + // These events update block metadata or grids the transcript reads. // PTY output redraws are driven by `wakeups_rx` below. ctx.subscribe_to_model(&model_events, |_, _, event, ctx| match event { @@ -167,6 +193,55 @@ impl TuiTerminalSessionView { controller.send_user_query_in_conversation(prompt, conversation_id, None, ctx); }); } + + /// Bridges shared shell-tool executor events into terminal-manager PTY intents. + fn handle_shell_command_executor_event( + &mut self, + event: &ShellCommandExecutorEvent, + model: &std::sync::Arc>, + ctx: &mut ViewContext, + ) { + match event { + ShellCommandExecutorEvent::ExecuteCommand { action_id, command } => { + let Some((session_id, conversation_id)) = (|| { + let model = model.lock(); + let session_id = model.block_list().active_block().session_id()?; + let conversation_id = BlocklistAIHistoryModel::as_ref(ctx) + .conversation_id_for_action(action_id, ctx.view_id())?; + Some((session_id, conversation_id)) + })() else { + log::warn!( + "Unable to execute TUI agent-requested command for action {action_id:?}" + ); + return; + }; + + ctx.emit(TuiTerminalSessionEvent::ExecuteCommand(Box::new( + ExecuteCommandEvent { + command: command.clone(), + session_id, + workflow_id: None, + workflow_command: None, + should_add_command_to_history: true, + source: CommandExecutionSource::AI { + metadata: AgentInteractionMetadata::new_hidden( + action_id.clone(), + conversation_id, + ), + }, + }, + ))); + } + ShellCommandExecutorEvent::WriteToPty { input, mode } => { + ctx.emit(TuiTerminalSessionEvent::WriteAgentInput { + bytes: Cow::Owned(input.to_vec()), + mode: *mode, + }); + } + ShellCommandExecutorEvent::CancelExecution + | ShellCommandExecutorEvent::TransferControlToUser { .. } => {} + } + } } impl Entity for TuiTerminalSessionView { @@ -216,3 +291,26 @@ impl TerminalSurface for TuiTerminalSessionView { ctx.notify(); } } + +#[cfg(test)] +mod tests { + use std::borrow::Cow; + + use warp::tui_export::{AIAgentPtyWriteMode, PtyIntent, PtyIntentEvent}; + + use super::TuiTerminalSessionEvent; + + #[test] + fn write_agent_input_event_maps_to_pty_intent() { + let event = TuiTerminalSessionEvent::WriteAgentInput { + bytes: Cow::Borrowed(b"hello"), + mode: AIAgentPtyWriteMode::Line, + }; + + let Some(PtyIntent::WriteAgentInput { bytes, mode }) = event.pty_intent() else { + panic!("expected write-agent-input PTY intent"); + }; + assert_eq!(&*bytes, b"hello"); + assert_eq!(mode, AIAgentPtyWriteMode::Line); + } +} diff --git a/specs/tui-agent-tool-calls/TECH.md b/specs/tui-agent-tool-calls/TECH.md index 9d9f3e1e84..9efaeace27 100644 --- a/specs/tui-agent-tool-calls/TECH.md +++ b/specs/tui-agent-tool-calls/TECH.md @@ -6,7 +6,7 @@ Relevant code at `526ade4522df0e65f138c67dcbcb90f1a3ce63e9`: - [`crates/warp_tui/src/terminal_session_view.rs`](https://github.com/warpdotdev/warp/blob/526ade4522df0e65f138c67dcbcb90f1a3ce63e9/crates/warp_tui/src/terminal_session_view.rs) — constructs the production AI stack for the TUI surface: `ActiveSession`, `TuiConversationSelection`, `BlocklistAIContextModel`, `BlocklistAIInputModel`, `BlocklistAIActionModel`, and `BlocklistAIController`. - [`crates/warp_tui/src/transcript_view.rs`](https://github.com/warpdotdev/warp/blob/526ade4522df0e65f138c67dcbcb90f1a3ce63e9/crates/warp_tui/src/transcript_view.rs) — subscribes to `BlocklistAIHistoryModel`, creates `TuiAgentBlockView`s for visible exchanges, and appends them to `TerminalModel::BlockList` as `RichContentType::AIBlock`. - [`crates/warp_tui/src/tui_block_list_viewport_source.rs`](https://github.com/warpdotdev/warp/blob/526ade4522df0e65f138c67dcbcb90f1a3ce63e9/crates/warp_tui/src/tui_block_list_viewport_source.rs) — walks the canonical block-list sum tree, measures dirty rich-content views, updates cached heights, and renders terminal blocks plus TUI agent blocks. -- [`crates/warp_tui/src/agent_block.rs`](https://github.com/warpdotdev/warp/blob/526ade4522df0e65f138c67dcbcb90f1a3ce63e9/crates/warp_tui/src/agent_block.rs) — currently derives `AgentBlockContent` from `output.text_from_agent_output()`, which only yields `AIAgentOutputMessageType::Text` and therefore drops action/tool-call messages. +- [`crates/warp_tui/src/agent_block.rs`](https://github.com/warpdotdev/warp/blob/526ade4522df0e65f138c67dcbcb90f1a3ce63e9/crates/warp_tui/src/agent_block.rs) — currently derives `TuiAgentBlockSection`s from `output.text_from_agent_output()`, which only yields `AIAgentOutputMessageType::Text` and therefore drops action/tool-call messages. - [`app/src/ai/agent/mod.rs`](https://github.com/warpdotdev/warp/blob/526ade4522df0e65f138c67dcbcb90f1a3ce63e9/app/src/ai/agent/mod.rs) — defines `AIAgentOutput`, ordered `AIAgentOutputMessage`s, `AIAgentOutputMessageType::Action(AIAgentAction)`, and helper iterators such as `text_from_agent_output()` and `actions()`. - [`app/src/ai/blocklist/block/view_impl/output.rs`](https://github.com/warpdotdev/warp/blob/526ade4522df0e65f138c67dcbcb90f1a3ce63e9/app/src/ai/blocklist/block/view_impl/output.rs) — GUI AI block rendering loops over `output.messages` in order and delegates each message variant to the appropriate renderer. - [`app/src/ai/blocklist/controller.rs`](https://github.com/warpdotdev/warp/blob/526ade4522df0e65f138c67dcbcb90f1a3ce63e9/app/src/ai/blocklist/controller.rs) — when a response stream finishes, queues emitted actions through `BlocklistAIActionModel::queue_actions` and later sends completed action results back to the model. @@ -32,59 +32,50 @@ The rendering path and the execution path share the same `AIAgentOutputMessageTy - Rendering derives a transcript item from the stored output message. - Execution is driven by `BlocklistAIController` and `BlocklistAIActionModel`. - Completed action results flow back to the LLM as `AIAgentInput::ActionResult`; the first TUI UI does not render those results. -### Ordered message-to-item adapter -Replace `TuiAgentBlockView::content`'s output extraction with an ordered adapter over `AIAgentOutput.messages`. The GUI AI block already follows this shape in `output.rs`: one ordered pass over messages, then variant-specific rendering. The TUI should match the ordering pattern without porting the GUI renderer. +### Ordered message-to-section adapter +Replace `TuiAgentBlockView::sections`' output extraction with an ordered pass over `AIAgentOutput.messages`. The GUI AI block already follows this shape in `output.rs`: one ordered pass over messages, then variant-specific rendering. The TUI matches the ordering pattern without porting the GUI renderer. ```mermaid flowchart LR Output["AIAgentOutput"] --> Messages["messages
server order"] - Messages --> Adapter["TuiAgentBlockView::content"] - Adapter --> Input["Input(String)"] - Adapter --> Text["Message::PlainText(String)"] - Adapter --> Tool["Message::ToolCall(TuiToolCallItem)"] - Tool --> ToolRender["TuiToolCallItem::render"] - ToolRender --> Label["TuiText
executed a tool call"] - Input --> Render["AgentBlockContent::render"] + Messages --> Adapter["TuiAgentBlockView::sections"] + Adapter --> Input["Input(Vec)"] + Adapter --> Text["PlainText(String)"] + Adapter --> Tool["ToolCall(Box)"] + Input --> Render["TuiAgentBlockSection::render_element"] Text --> Render - Label --> Render + Tool --> Render + Render --> Label["TuiText
executed a tool call"] ``` -Use a two-level render model so the block stays extensible as more message types get TUI renderers: +Use a flat section enum so the block stays extensible as more message types get TUI renderers: ```rust -#[derive(Debug, Eq, PartialEq)] -enum TuiAgentBlockItem { - Input(String), - Message(TuiAgentMessageItem), -} - -#[derive(Debug, Eq, PartialEq)] -enum TuiAgentMessageItem { +#[derive(Clone, Debug, Eq, PartialEq)] +enum TuiAgentBlockSection { + Input(Vec), PlainText(String), - ToolCall(TuiToolCallItem), -} - -#[derive(Debug, Eq, PartialEq)] -struct TuiToolCallItem { - action: AIAgentAction, + ToolCall(Box), } ``` -`TuiToolCallItem` should carry the full `AIAgentAction` even though the first renderer ignores the fields. The item is derived render data, not durable UI state. Carrying the action now gives future renderers direct access to `id`, `task_id`, `requires_result`, and the concrete `AIAgentActionType` without changing the adapter API. +The `ToolCall` variant carries the full `AIAgentAction` even though the first renderer ignores the fields. The section is derived render data, not durable UI state. Carrying the action now gives future renderers direct access to `id`, `task_id`, `requires_result`, and the concrete `AIAgentActionType` without changing the adapter API. `AIAgentAction` is large, so the variant is boxed to satisfy clippy's `large_enum_variant`. The adapter behavior should be: -- `AIAgentInput::display_query()` values still become `TuiAgentBlockItem::Input`. -- `AIAgentOutputMessageType::Text(AIAgentText { sections })` becomes one `Message(PlainText(...))` per non-empty `AIAgentTextSection::PlainText`. -- `AIAgentOutputMessageType::Action(action)` becomes one `Message(ToolCall(TuiToolCallItem { action: action.clone() }))`. +- `AIAgentInput::display_query()` values become `TuiAgentBlockSection::Input`, split into one entry per line. +- `AIAgentOutputMessageType::Text(AIAgentText { sections })` becomes one `TuiAgentBlockSection::PlainText` per non-empty `AIAgentTextSection::PlainText`. +- `AIAgentOutputMessageType::Action(action)` becomes one `TuiAgentBlockSection::ToolCall(Box::new(action.clone()))`. - Code, table, image, Mermaid, reasoning, summarization, todo, subagent, web, artifact, skill, message-bus, lifecycle-event, debug, and comments-addressed messages remain unsupported until the TUI has specific renderers for them. -`TuiToolCallItem::render` should render exactly `executed a tool call`. The first implementation should not include the tool name, status, arguments, or result details. +The `ToolCall` arm of `TuiAgentBlockSection::render_element` renders exactly `executed a tool call`. The first implementation does not include the tool name, status, arguments, or result details. The stub is styled as a muted status row (`theme.terminal_colors().bright.black` plus `Modifier::DIM`) so it reads as a tool-call event rather than blending into the agent's plain-text prose. ### State ownership -Do not store a materialized `Vec` on `TuiAgentBlockView`. Like the GUI AI block, durable state should be stored only when it represents interaction state that must survive renders. -For this first renderer, `TuiAgentBlockView` only needs: +Do not store a materialized `Vec` on `TuiAgentBlockView`. Like the GUI AI block, durable state should be stored only when it represents interaction state that must survive renders. +`TuiAgentBlockView` holds the exchange identity and backing model; sections are re-derived on each render: ```rust pub(super) struct TuiAgentBlockView { + conversation_id: AIConversationId, + exchange_id: AIAgentExchangeId, model: Rc>, } ``` Later stateful renderers can add maps keyed by stable IDs: - `MessageId` for collapsible reasoning, summarization, web, or todo sections. - `AIAgentActionId` for expandable or status-aware tool cards. -Even when those maps exist, `output.messages` should remain the ordering source. The render adapter can consult state maps while deriving items, but it should not replace the ordered message pass with independently ordered per-type collections. +Even when those maps exist, `output.messages` should remain the ordering source. The render adapter can consult state maps while deriving sections, but it should not replace the ordered message pass with independently ordered per-type collections. ### Redraw and height updates No action-model event subscription is needed for the static tool-call label. New action messages enter the exchange output through the existing response-stream/history path. `TuiTranscriptView::mark_exchange_dirty` already marks the owning rich-content view dirty on `UpdatedStreamingExchange`, and `TuiBlockListViewportSource` already measures dirty rich-content views and writes updated heights back to the terminal block list. ```mermaid @@ -139,7 +130,7 @@ Add a shared, non-GUI fallback inside `RequestFileEditsExecutor`: This is not a TUI renderer concern. It belongs in the shared executor so future headless or non-GUI surfaces can execute file edits without depending on GUI views. ### Public TUI export boundary Extend `app/src/tui_export.rs` only for types that `warp_tui` must name directly: -- `AIAgentAction`, `AIAgentActionId`, `AIAgentActionType`, and `TaskId` for `TuiToolCallItem` and test fixtures. +- `AIAgentAction`, `AIAgentActionId`, `AIAgentActionType`, and `TaskId` for the `ToolCall` section variant and test fixtures. - `AIAgentPtyWriteMode`, `AgentInteractionMetadata`, and `ShellCommandExecutorEvent` for the PTY bridge. Do not export action status/result types for this feature. The static stub renderer does not need them. ### Boundaries and non-goals @@ -150,15 +141,14 @@ Do not change canonical transcript ordering. Agent blocks remain `RichContentTyp ## Testing and validation Add unit tests in `crates/warp_tui/src/agent_block_tests.rs` for the ordered message adapter: - `Text -> Action -> Text` renders as text, `executed a tool call`, text in that order. -- Multiple action messages render multiple `TuiToolCallItem` stub lines. -- `TuiToolCallItem` preserves the action ID and action type in derived content even though rendering only emits the static label. +- Multiple action messages render multiple `ToolCall` stub lines. +- The `ToolCall` section preserves the action ID and action type in derived content even though rendering only emits the static label. - Unsupported text sections and unsupported message variants are ignored without disturbing adjacent supported items. - `desired_height` accounts for the tool-call stub line. Keep transcript dirty/reflow tests focused on `UpdatedStreamingExchange`; no action-model event dirtying test is needed for the static stub. Add shell bridge tests: -- `TuiTerminalSessionEvent::ExecuteCommand` maps to `PtyIntent::ExecuteCommand`. - `TuiTerminalSessionEvent::WriteAgentInput` maps to `PtyIntent::WriteAgentInput`. -- Shell executor event translation emits an AI-sourced `ExecuteCommandEvent` when an active session and owning conversation can be found. +- The `ExecuteCommand` → `PtyIntent::ExecuteCommand` branch and the shell-executor event translation are covered by compilation only, because constructing an `ExecuteCommandEvent` requires `SessionId`, which is intentionally not exported through `tui_export` for this feature. Add direct tests for these when a session-id test seam is available. Add or port `RequestFileEditsExecutor` tests for the no-`CodeDiffView` path: - Prepared diff success saves files and returns `RequestFileEditsResult::Success`. - Save failure returns `DiffApplicationFailed`. From 5693b525afb95509dc845f1c838ebd0d584cbc0c Mon Sep 17 00:00:00 2001 From: harryalbert Date: Wed, 1 Jul 2026 14:09:37 -0400 Subject: [PATCH 3/6] revert file editing (for now) --- .../execute/request_file_edits.rs | 245 +++--------------- .../execute/request_file_edits_tests.rs | 95 +------ specs/tui-agent-tool-calls/TECH.md | 23 +- 3 files changed, 39 insertions(+), 324 deletions(-) diff --git a/app/src/ai/blocklist/action_model/execute/request_file_edits.rs b/app/src/ai/blocklist/action_model/execute/request_file_edits.rs index e93b2391f1..f75a01e0ab 100644 --- a/app/src/ai/blocklist/action_model/execute/request_file_edits.rs +++ b/app/src/ai/blocklist/action_model/execute/request_file_edits.rs @@ -3,11 +3,9 @@ mod diff_application; mod telemetry; use std::collections::HashMap; -use std::fs; -use std::ops::Range; -use std::path::{Path, PathBuf}; +use std::path::PathBuf; -use ai::diff_validation::{AIRequestedCodeDiff, DiffDelta, DiffType}; +use ai::diff_validation::AIRequestedCodeDiff; use apply_diff_model::ApplyDiffModel; use diff_application::DiffApplicationError; pub(crate) use diff_application::{apply_edits, FileReadResult}; @@ -15,7 +13,6 @@ use futures::channel::oneshot; use futures::future::BoxFuture; use futures::FutureExt; use itertools::Itertools; -use similar::{DiffOp, TextDiff}; pub(crate) use telemetry::MalformedFinalLineProxyEvent; #[allow(unused_imports)] pub use telemetry::{EditAcceptAndContinueClickedEvent, EditAcceptClickedEvent}; @@ -40,7 +37,6 @@ use crate::ai::blocklist::inline_action::code_diff_view::{ }; use crate::ai::blocklist::{BlocklistAIPermissions, RequestedEditResolution}; use crate::ai::paths::host_native_absolute_path; -use crate::code::DiffResult; use crate::terminal::model::session::active_session::ActiveSession; use crate::terminal::model::session::SessionType; use crate::{safe_warn, BlocklistAIHistoryModel}; @@ -50,8 +46,6 @@ pub struct RequestFileEditsExecutor { active_session: ModelHandle, apply_diff_model: ModelHandle, diff_views: HashMap>, - /// Prepared diffs keyed by action ID, used by non-GUI run-to-completion surfaces. - prepared_diffs: HashMap>, /// Set of action IDs where diff application failed. diff_application_failures: HashMap>, terminal_view_id: EntityId, @@ -68,7 +62,6 @@ impl RequestFileEditsExecutor { active_session, apply_diff_model, diff_views: HashMap::new(), - prepared_diffs: HashMap::new(), diff_application_failures: HashMap::new(), terminal_view_id, } @@ -153,6 +146,14 @@ impl RequestFileEditsExecutor { return ActionExecution::InvalidAction; }; + // TODO(surface-agnostic-file-edit-execution): non-GUI surfaces (e.g. the TUI) have no + // CodeDiffView, so file-edit tool calls are not executable here yet. The stacked + // surface-agnostic refactor routes execution through a shared PersistDiffModel instead. + let Some(diff_view) = self.diff_views.get(id) else { + log::warn!("Tried to execute a RequestFileEdits action without a diff view"); + return ActionExecution::NotReady; + }; + // If diff application failed, early exit. if let Some(errors) = self.diff_application_failures.remove(id) { return ActionExecution::Sync(AIAgentActionResultType::RequestFileEdits( @@ -162,16 +163,6 @@ impl RequestFileEditsExecutor { )); } - let Some(diff_view) = self.diff_views.get(id) else { - let Some(prepared_diffs) = self.prepared_diffs.remove(id) else { - log::warn!("Tried to execute a RequestFileEdits action without prepared diffs"); - return ActionExecution::NotReady; - }; - return ActionExecution::Sync(AIAgentActionResultType::RequestFileEdits( - save_prepared_diffs(prepared_diffs), - )); - }; - let identifiers = self .generate_ai_identifiers(&input.conversation_id, id, ctx) .unwrap_or_else(|| AIIdentifiers { @@ -329,6 +320,13 @@ impl RequestFileEditsExecutor { ) { tx.send(()).ok(); + let Some(diff_view) = self.diff_views.get(&id) else { + log::warn!( + "Tried to apply diffs for a RequestFileEdits action without a corresponding diff view" + ); + return; + }; + let applied_diffs = match applied_diffs { Ok(diffs) if !diffs.is_empty() => diffs, Ok(_) => { @@ -367,23 +365,19 @@ impl RequestFileEditsExecutor { diffs.push(file_diff); } - self.prepared_diffs.insert(id.clone(), diffs.clone()); - - if let Some(diff_view) = self.diff_views.get(&id) { - // Set the session type on the diff view so save/delete/create routes - // through the correct FileModel backend. - let diff_session_type = match self.active_session.as_ref(ctx).session_type(ctx) { - Some(SessionType::WarpifiedRemote { - host_id: Some(host_id), - }) => DiffSessionType::Remote(host_id.clone()), - _ => DiffSessionType::Local, - }; - - diff_view.update(ctx, |diff_view, ctx| { - diff_view.set_diff_session_type(diff_session_type); - diff_view.set_candidate_diffs(diffs, ctx); - }); - } + // Set the session type on the diff view so save/delete/create routes + // through the correct FileModel backend. + let diff_session_type = match self.active_session.as_ref(ctx).session_type(ctx) { + Some(SessionType::WarpifiedRemote { + host_id: Some(host_id), + }) => DiffSessionType::Remote(host_id.clone()), + _ => DiffSessionType::Local, + }; + + diff_view.update(ctx, |diff_view, ctx| { + diff_view.set_diff_session_type(diff_session_type); + diff_view.set_candidate_diffs(diffs, ctx); + }); } fn generate_ai_identifiers( @@ -479,185 +473,6 @@ fn updated_file_contexts_from_editor_buffers( .collect() } -/// Saves prepared diffs without requiring a GUI review view. -fn save_prepared_diffs(prepared_diffs: Vec) -> RequestFileEditsResult { - let mut combined_diff = DiffResult::default(); - let mut updated_files = Vec::new(); - let mut deleted_files = Vec::new(); - let mut content_map = HashMap::new(); - - for diff in prepared_diffs { - match save_prepared_diff(diff) { - Ok(saved) => { - combined_diff += &saved.diff; - if let Some(content) = saved.content { - content_map.insert(saved.file_location.name.clone(), content); - updated_files.push((saved.file_location, false)); - } - deleted_files.extend(saved.deleted_files); - } - Err(error) => { - return RequestFileEditsResult::DiffApplicationFailed { error }; - } - } - } - - RequestFileEditsResult::Success { - diff: combined_diff.unified_diff, - updated_files: updated_file_contexts_from_editor_buffers(&updated_files, &content_map), - deleted_files, - lines_added: combined_diff.lines_added, - lines_removed: combined_diff.lines_removed, - } -} - -struct SavedPreparedDiff { - diff: DiffResult, - file_location: FileLocations, - content: Option, - deleted_files: Vec, -} - -fn save_prepared_diff(diff: FileDiff) -> Result { - let path = diff.file_path(); - let mut deleted_files = Vec::new(); - let (target_path, next_content) = match &diff.diff_type { - DiffType::Create { delta } => (path.clone(), delta.insertion.clone()), - DiffType::Update { deltas, rename } => { - let target_path = rename - .as_ref() - .map(|rename| rename.to_string_lossy().to_string()) - .unwrap_or_else(|| path.clone()); - let next_content = apply_deltas_to_content(&diff.base.content, deltas)?; - if target_path != path { - deleted_files.push(path.clone()); - } - (target_path, next_content) - } - DiffType::Delete { .. } => { - fs::remove_file(&path).map_err(|error| format!("Failed to delete {path}: {error}"))?; - return Ok(SavedPreparedDiff { - diff: diff_result(&diff.base.content, "", &path), - file_location: FileLocations { - name: path.clone(), - lines: vec![], - }, - content: None, - deleted_files: vec![path], - }); - } - }; - - if let Some(parent) = Path::new(&target_path).parent() { - fs::create_dir_all(parent) - .map_err(|error| format!("Failed to create parent directory {parent:?}: {error}"))?; - } - fs::write(&target_path, &next_content) - .map_err(|error| format!("Failed to write {target_path}: {error}"))?; - if target_path != path { - fs::remove_file(&path).map_err(|error| format!("Failed to remove {path}: {error}"))?; - } - - Ok(SavedPreparedDiff { - diff: diff_result(&diff.base.content, &next_content, &target_path), - file_location: FileLocations { - name: target_path, - lines: changed_lines_for_headless_result(&diff.diff_type), - }, - content: Some(next_content), - deleted_files, - }) -} - -fn apply_deltas_to_content(content: &str, deltas: &[DiffDelta]) -> Result { - let mut lines = split_lines_preserving_newlines(content); - let mut deltas = deltas.to_vec(); - deltas.sort_by_key(|delta| delta.replacement_line_range.start); - - for delta in deltas.into_iter().rev() { - let start = delta.replacement_line_range.start.saturating_sub(1); - let end = delta.replacement_line_range.end.saturating_sub(1); - if start > lines.len() || end > lines.len() || start > end { - return Err(format!( - "Diff range {:?} is out of bounds for file with {} lines", - delta.replacement_line_range, - lines.len() - )); - } - let replacement = split_lines_preserving_newlines(&delta.insertion); - lines.splice(start..end, replacement); - } - - Ok(lines.concat()) -} - -fn split_lines_preserving_newlines(content: &str) -> Vec { - if content.is_empty() { - Vec::new() - } else { - content.split_inclusive('\n').map(str::to_string).collect() - } -} - -fn diff_result(before: &str, after: &str, file_name: &str) -> DiffResult { - if before == after { - return DiffResult::default(); - } - - let text_diff = TextDiff::from_lines(before, after); - let mut lines_added = 0; - let mut lines_removed = 0; - for op in text_diff.ops() { - match op { - DiffOp::Equal { .. } => {} - DiffOp::Delete { old_len, .. } => lines_removed += old_len, - DiffOp::Insert { new_len, .. } => lines_added += new_len, - DiffOp::Replace { - old_len, new_len, .. - } => { - lines_removed += old_len; - lines_added += new_len; - } - } - } - - DiffResult { - unified_diff: text_diff - .unified_diff() - .context_radius(3) - .header(file_name, file_name) - .missing_newline_hint(false) - .to_string(), - lines_added, - lines_removed, - } -} - -fn changed_lines_for_headless_result(diff_type: &DiffType) -> Vec> { - match diff_type { - DiffType::Create { delta } => inserted_content_range(1, &delta.insertion) - .into_iter() - .collect(), - DiffType::Update { deltas, .. } => deltas - .iter() - .filter_map(changed_line_range_for_delta) - .collect(), - DiffType::Delete { .. } => vec![], - } -} - -fn changed_line_range_for_delta(delta: &DiffDelta) -> Option> { - let replacement_range = &delta.replacement_line_range; - if replacement_range.start == replacement_range.end { - return inserted_content_range(replacement_range.start.max(1), &delta.insertion); - } - Some(replacement_range.clone()) -} - -fn inserted_content_range(start: usize, content: &str) -> Option> { - let line_count = content.lines().count(); - (line_count > 0).then_some(start..start + line_count) -} fn clamp_to_file_context_range_start(file_location: &mut FileLocations) { for range in &mut file_location.lines { range.start = range.start.max(1); diff --git a/app/src/ai/blocklist/action_model/execute/request_file_edits_tests.rs b/app/src/ai/blocklist/action_model/execute/request_file_edits_tests.rs index 0bfad07b8b..3ea2a50eaf 100644 --- a/app/src/ai/blocklist/action_model/execute/request_file_edits_tests.rs +++ b/app/src/ai/blocklist/action_model/execute/request_file_edits_tests.rs @@ -1,12 +1,9 @@ use std::collections::HashMap; -use std::fs; -use ai::agent::action_result::{AnyFileContent, RequestFileEditsResult}; +use ai::agent::action_result::AnyFileContent; use ai::agent::FileLocations; -use ai::diff_validation::{DiffDelta, DiffType}; -use super::{save_prepared_diffs, updated_file_contexts_from_editor_buffers}; -use crate::ai::blocklist::inline_action::code_diff_view::FileDiff; +use super::updated_file_contexts_from_editor_buffers; #[test] fn updated_file_contexts_from_editor_buffers_returns_changed_lines_with_context() { @@ -41,94 +38,6 @@ fn updated_file_contexts_from_editor_buffers_returns_changed_lines_with_context( ); } -#[test] -fn save_prepared_diffs_creates_file_without_code_diff_view() { - let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("src").join("main.rs"); - let path = path.to_string_lossy().to_string(); - - let result = save_prepared_diffs(vec![FileDiff::new( - String::new(), - path.clone(), - DiffType::creation("fn main() {}\n".to_owned()), - )]); - - let RequestFileEditsResult::Success { - updated_files, - deleted_files, - lines_added, - lines_removed, - .. - } = result - else { - panic!("expected create diff to save successfully"); - }; - - assert_eq!(fs::read_to_string(&path).unwrap(), "fn main() {}\n"); - assert_eq!(deleted_files, Vec::::new()); - assert_eq!(lines_added, 1); - assert_eq!(lines_removed, 0); - assert_eq!(updated_files.len(), 1); - assert_eq!(updated_files[0].file_context.file_name, path); -} - -#[test] -fn save_prepared_diffs_updates_file_without_code_diff_view() { - let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("main.rs"); - let path = path.to_string_lossy().to_string(); - let original = "one\ntwo\nthree\n"; - fs::write(&path, original).unwrap(); - - let result = save_prepared_diffs(vec![FileDiff::new( - original.to_owned(), - path.clone(), - DiffType::update( - vec![DiffDelta { - replacement_line_range: 2..3, - insertion: "TWO\n".to_owned(), - }], - None, - ), - )]); - - let RequestFileEditsResult::Success { - updated_files, - lines_added, - lines_removed, - .. - } = result - else { - panic!("expected update diff to save successfully"); - }; - - assert_eq!(fs::read_to_string(&path).unwrap(), "one\nTWO\nthree\n"); - assert_eq!(lines_added, 1); - assert_eq!(lines_removed, 1); - assert_eq!(updated_files.len(), 1); - assert_eq!(updated_files[0].file_context.file_name, path); -} - -#[test] -fn save_prepared_diffs_reports_save_failure_without_code_diff_view() { - let dir = tempfile::tempdir().unwrap(); - let path = dir.path().to_string_lossy().to_string(); - - let result = save_prepared_diffs(vec![FileDiff::new( - String::new(), - path, - DiffType::creation("not a directory write\n".to_owned()), - )]); - - let RequestFileEditsResult::DiffApplicationFailed { error } = result else { - panic!("expected save failure"); - }; - assert!( - error.contains("Failed to write"), - "unexpected error message: {error}" - ); -} - #[test] fn updated_file_contexts_from_editor_buffers_preserves_full_file_when_no_ranges() { let updated_files = vec![( diff --git a/specs/tui-agent-tool-calls/TECH.md b/specs/tui-agent-tool-calls/TECH.md index 9efaeace27..8011242aca 100644 --- a/specs/tui-agent-tool-calls/TECH.md +++ b/specs/tui-agent-tool-calls/TECH.md @@ -120,14 +120,9 @@ Subscribe `TuiTerminalSessionView` to `action_model.as_ref(ctx).shell_command_ex - `ShellCommandExecutorEvent::WriteToPty { input, mode }` into `TuiTerminalSessionEvent::WriteAgentInput`. - `CancelExecution` and `TransferControlToUser { .. }` as no-ops for the first pass. The command event should use `CommandExecutionSource::AI` and `AgentInteractionMetadata::new_hidden(action_id, conversation_id)`. The conversation ID should be looked up with `BlocklistAIHistoryModel::conversation_id_for_action(action_id, ctx.view_id())`, and the active session ID should come from the current terminal model active block. -### Shared headless file-edit execution -`RequestFileEdits` currently depends on a GUI `CodeDiffView`: without a registered diff view, `RequestFileEditsExecutor::execute` returns `NotReady`. The TUI should not create a GUI diff view just to make file-edit tool calls executable. -Add a shared, non-GUI fallback inside `RequestFileEditsExecutor`: -- During `preprocess_action`, if diff application succeeds and no `CodeDiffView` is registered, store prepared diffs by `AIAgentActionId`. -- During `execute`, prefer the existing GUI `CodeDiffView` path when a view is registered. -- If no view is registered but prepared diffs exist, save those diffs through the same local/remote file backend semantics used by the GUI path and return `RequestFileEditsResult::Success`. -- If preprocessing failed, return `RequestFileEditsResult::DiffApplicationFailed` so the LLM can recover in a follow-up. -This is not a TUI renderer concern. It belongs in the shared executor so future headless or non-GUI surfaces can execute file edits without depending on GUI views. +### Headless file-edit execution (deferred) +`RequestFileEdits` renders in the transcript like other tool calls, but it is not executed on non-GUI surfaces in this branch: `RequestFileEditsExecutor::execute` still requires a registered GUI `CodeDiffView` and returns `NotReady` otherwise (`app/src/ai/blocklist/action_model/execute/request_file_edits.rs`). +Making file edits executable without a GUI review view is handled in the stacked `surface-agnostic-file-edit-execution` follow-up, which routes both surfaces through a shared, non-GUI `PersistDiffModel`. This branch deliberately adds no headless persistence path. ### Public TUI export boundary Extend `app/src/tui_export.rs` only for types that `warp_tui` must name directly: - `AIAgentAction`, `AIAgentActionId`, `AIAgentActionType`, and `TaskId` for the `ToolCall` section variant and test fixtures. @@ -149,10 +144,7 @@ Keep transcript dirty/reflow tests focused on `UpdatedStreamingExchange`; no act Add shell bridge tests: - `TuiTerminalSessionEvent::WriteAgentInput` maps to `PtyIntent::WriteAgentInput`. - The `ExecuteCommand` → `PtyIntent::ExecuteCommand` branch and the shell-executor event translation are covered by compilation only, because constructing an `ExecuteCommandEvent` requires `SessionId`, which is intentionally not exported through `tui_export` for this feature. Add direct tests for these when a session-id test seam is available. -Add or port `RequestFileEditsExecutor` tests for the no-`CodeDiffView` path: -- Prepared diff success saves files and returns `RequestFileEditsResult::Success`. -- Save failure returns `DiffApplicationFailed`. -- Preprocessing failure returns `DiffApplicationFailed`. +File-edit execution is out of scope for this branch (deferred to the stacked follow-up), so no `RequestFileEditsExecutor` persistence tests are added here. Run targeted tests first: ```bash cargo test -p warp_tui agent_block @@ -166,15 +158,14 @@ Then run formatting and the standard Rust validation for touched crates: cargo clippy --workspace --all-targets --all-features --tests -- -D warnings ``` ## Parallelization -Do not use child agents for the implementation. The work is tightly coupled across one TUI surface, one render adapter, shared executor exports, and one shared executor fallback. Splitting implementation across worktrees would create more merge overhead than wall-clock savings. +Do not use child agents for the implementation. The work is tightly coupled across one TUI surface, one render adapter, and shared executor exports. Splitting implementation across worktrees would create more merge overhead than wall-clock savings. The implementation sequence should be: ```mermaid flowchart LR A["TUI exports"] --> B["Ordered TUI render adapter"] B --> C["Autoexecute selection constructor"] C --> D["Shell PTY bridge"] - D --> E["Headless file-edit fallback"] - E --> F["Targeted tests"] + D --> F["Targeted tests"] F --> G["Format + clippy"] ``` -The shell bridge and file-edit fallback can be reviewed as separate conceptual units, but they should land in one branch because the user-facing feature is one TUI tool-call execution path. +The shell bridge lands in this branch; headless file-edit execution is a separate stacked follow-up because it primarily refactors GUI persistence rather than the TUI transcript path. From 29f9aa12f7d2a5fd20d7f01384bfb95e1350cf86 Mon Sep 17 00:00:00 2001 From: harryalbert Date: Wed, 1 Jul 2026 16:04:56 -0400 Subject: [PATCH 4/6] improve comments and test --- crates/warp_tui/src/agent_block.rs | 23 ++++++---- crates/warp_tui/src/conversation_selection.rs | 19 ++------ .../src/conversation_selection_tests.rs | 20 +++++--- crates/warp_tui/src/terminal_session_view.rs | 36 ++++----------- specs/tui-agent-tool-calls/TECH.md | 46 +++++++++---------- 5 files changed, 62 insertions(+), 82 deletions(-) diff --git a/crates/warp_tui/src/agent_block.rs b/crates/warp_tui/src/agent_block.rs index 4e242fd80c..db05a97cde 100644 --- a/crates/warp_tui/src/agent_block.rs +++ b/crates/warp_tui/src/agent_block.rs @@ -112,14 +112,16 @@ impl TuiAIBlock { for message in &output.messages { match &message.message { AIAgentOutputMessageType::Text(text) => { - sections.extend(text.sections.iter().filter_map(|section| match section { - AIAgentTextSection::PlainText { text } => (!text.text().is_empty()) - .then(|| TuiAIBlockSection::PlainText(text.text().to_owned())), - // Add item variants here as the TUI learns to render richer sections. - AIAgentTextSection::Code { .. } - | AIAgentTextSection::Table { .. } - | AIAgentTextSection::Image { .. } - | AIAgentTextSection::MermaidDiagram { .. } => None, + sections.extend(text.sections.iter().filter_map(|section| { + match section { + AIAgentTextSection::PlainText { text } => (!text.text().is_empty()) + .then(|| TuiAIBlockSection::PlainText(text.text().to_owned())), + // Add item variants here as the TUI learns to render richer sections. + AIAgentTextSection::Code { .. } + | AIAgentTextSection::Table { .. } + | AIAgentTextSection::Image { .. } + | AIAgentTextSection::MermaidDiagram { .. } => None, + } })); } AIAgentOutputMessageType::Action(action) => { @@ -215,12 +217,15 @@ impl TuiAIBlockSection { .finish() } Self::ToolCall(_action) => { + // TODO: add richer rendering for each tool call type. This is just a rendering stub to build off of. let text_color = Fill::from(ThemeFill::from(theme.terminal_colors().bright.black)).into(); Box::new( TuiContainer::new( TuiText::new("executed a tool call").with_style( - TuiStyle::default().fg(text_color).add_modifier(Modifier::DIM), + TuiStyle::default() + .fg(text_color) + .add_modifier(Modifier::DIM), ), ) .with_padding_top(top_padding), diff --git a/crates/warp_tui/src/conversation_selection.rs b/crates/warp_tui/src/conversation_selection.rs index 5d7dbe7ed0..02f1181165 100644 --- a/crates/warp_tui/src/conversation_selection.rs +++ b/crates/warp_tui/src/conversation_selection.rs @@ -14,22 +14,7 @@ pub(super) struct TuiConversationSelection { impl TuiConversationSelection { /// Creates TUI conversation selection for a terminal surface. - #[cfg(test)] pub(super) fn new( - terminal_surface_id: EntityId, - ctx: &mut ModelContext>, - ) -> Self { - let default_autoexecute_override = - if warp_core::execution_mode::AppExecutionMode::as_ref(ctx).is_sandboxed() { - AIConversationAutoexecuteMode::RunToCompletion - } else { - AIConversationAutoexecuteMode::RespectUserSettings - }; - Self::new_with_autoexecute_override(terminal_surface_id, default_autoexecute_override, ctx) - } - - /// Creates TUI conversation selection with an explicit new-conversation autoexecute mode. - pub(super) fn new_with_autoexecute_override( terminal_surface_id: EntityId, autoexecute_override: AIConversationAutoexecuteMode, ctx: &mut ModelContext>, @@ -38,9 +23,13 @@ impl TuiConversationSelection { &BlocklistAIHistoryModel::handle(ctx), |selection, _, event, ctx| selection.handle_history_event(event, ctx), ); + + // TODO: Implement actual permissions once settings are in place and there is a UI for permissions requests. + // For now, we just always set fast-forward to on. let pending_query_state = PendingQueryState::New { autoexecute_override, }; + Self { terminal_surface_id, default_autoexecute_override: autoexecute_override, diff --git a/crates/warp_tui/src/conversation_selection_tests.rs b/crates/warp_tui/src/conversation_selection_tests.rs index aab90b10c9..f69d20b82a 100644 --- a/crates/warp_tui/src/conversation_selection_tests.rs +++ b/crates/warp_tui/src/conversation_selection_tests.rs @@ -1,6 +1,6 @@ use warp::tui_export::{ - AIConversationId, AgentViewEntryOrigin, BlocklistAIHistoryEvent, BlocklistAIHistoryModel, - ConversationSelection, ConversationSelectionHandle, + AIConversationAutoexecuteMode, AIConversationId, AgentViewEntryOrigin, BlocklistAIHistoryEvent, + BlocklistAIHistoryModel, ConversationSelection, ConversationSelectionHandle, }; use warp_core::execution_mode::{AppExecutionMode, ExecutionMode}; use warpui::{App, EntityId, ModelHandle}; @@ -18,8 +18,11 @@ fn build_tui_selection( let history = app.add_singleton_model(|_| BlocklistAIHistoryModel::default()); let terminal_surface_id = EntityId::new(); let selection = app.add_model(|ctx| { - Box::new(TuiConversationSelection::new(terminal_surface_id, ctx)) - as Box + Box::new(TuiConversationSelection::new( + terminal_surface_id, + AIConversationAutoexecuteMode::RespectUserSettings, + ctx, + )) as Box }); (history, selection, terminal_surface_id) } @@ -126,12 +129,15 @@ fn tui_selection_reconciles_split_and_removed_selection() { #[test] fn tui_new_conversation_preserves_pending_autoexecute_override() { App::test((), |mut app| async move { - app.add_singleton_model(|ctx| AppExecutionMode::new(ExecutionMode::App, true, ctx)); + app.add_singleton_model(|ctx| AppExecutionMode::new(ExecutionMode::App, false, ctx)); let history = app.add_singleton_model(|_| BlocklistAIHistoryModel::default()); let terminal_surface_id = EntityId::new(); let selection = app.add_model(|ctx| { - Box::new(TuiConversationSelection::new(terminal_surface_id, ctx)) - as Box + Box::new(TuiConversationSelection::new( + terminal_surface_id, + AIConversationAutoexecuteMode::RunToCompletion, + ctx, + )) as Box }); let conversation_id = selection diff --git a/crates/warp_tui/src/terminal_session_view.rs b/crates/warp_tui/src/terminal_session_view.rs index 1e93b2e70f..5685744068 100644 --- a/crates/warp_tui/src/terminal_session_view.rs +++ b/crates/warp_tui/src/terminal_session_view.rs @@ -1,6 +1,8 @@ //! Authenticated terminal-session TUI surface. use std::borrow::Cow; +use std::sync::Arc; +use parking_lot::FairMutex; use warp::editor::CodeEditorModel; use warp::tui_export::{ AIAgentPtyWriteMode, AIConversationAutoexecuteMode, ActiveSession, AgentInteractionMetadata, @@ -8,7 +10,7 @@ use warp::tui_export::{ BlocklistAIController, BlocklistAIHistoryModel, BlocklistAIInputModel, CommandExecutionSource, ConversationSelection, ConversationSelectionHandle, ExecuteCommandEvent, GetRelevantFilesController, ModelEvent, PtyIntent, PtyIntentEvent, ShellCommandExecutorEvent, - TerminalSurface, TerminalSurfaceInit, + TerminalModel, TerminalSurface, TerminalSurfaceInit, }; use warp_core::ui::theme::Fill as ThemeFill; use warpui::SingletonEntity; @@ -71,9 +73,8 @@ impl TuiTerminalSessionView { let terminal_surface_id: EntityId = ctx.view_id(); let active_session = ctx.add_model(|ctx| ActiveSession::new(sessions.clone(), model_events.clone(), ctx)); - // Agent-requested commands run to completion so tool calls execute end to end. let conversation_selection = ctx.add_model(|ctx| { - Box::new(TuiConversationSelection::new_with_autoexecute_override( + Box::new(TuiConversationSelection::new( terminal_surface_id, AIConversationAutoexecuteMode::RunToCompletion, ctx, @@ -198,7 +199,7 @@ impl TuiTerminalSessionView { fn handle_shell_command_executor_event( &mut self, event: &ShellCommandExecutorEvent, - model: &std::sync::Arc>, + model: &Arc>, ctx: &mut ViewContext, ) { match event { @@ -238,6 +239,10 @@ impl TuiTerminalSessionView { mode: *mode, }); } + // TODO(tui-agent-cancel): we need to think about how we want to handle ctrl c. + // Right now it shuts down the entire app, but we should probably mimic the pattern from claude code, amp, etc. + // and have one ctrl c shut down any in progress conversation or tool call, and a double ctrl c actually close the app + // (with some ephemeral message after the first ctrl c). ShellCommandExecutorEvent::CancelExecution | ShellCommandExecutorEvent::TransferControlToUser { .. } => {} } @@ -291,26 +296,3 @@ impl TerminalSurface for TuiTerminalSessionView { ctx.notify(); } } - -#[cfg(test)] -mod tests { - use std::borrow::Cow; - - use warp::tui_export::{AIAgentPtyWriteMode, PtyIntent, PtyIntentEvent}; - - use super::TuiTerminalSessionEvent; - - #[test] - fn write_agent_input_event_maps_to_pty_intent() { - let event = TuiTerminalSessionEvent::WriteAgentInput { - bytes: Cow::Borrowed(b"hello"), - mode: AIAgentPtyWriteMode::Line, - }; - - let Some(PtyIntent::WriteAgentInput { bytes, mode }) = event.pty_intent() else { - panic!("expected write-agent-input PTY intent"); - }; - assert_eq!(&*bytes, b"hello"); - assert_eq!(mode, AIAgentPtyWriteMode::Line); - } -} diff --git a/specs/tui-agent-tool-calls/TECH.md b/specs/tui-agent-tool-calls/TECH.md index 8011242aca..74c2718d5b 100644 --- a/specs/tui-agent-tool-calls/TECH.md +++ b/specs/tui-agent-tool-calls/TECH.md @@ -4,9 +4,9 @@ The TUI transcript renders terminal blocks and simple AI exchange blocks in cano Relevant code at `526ade4522df0e65f138c67dcbcb90f1a3ce63e9`: - [`crates/warp_tui/src/session.rs`](https://github.com/warpdotdev/warp/blob/526ade4522df0e65f138c67dcbcb90f1a3ce63e9/crates/warp_tui/src/session.rs) — boots the headless app, creates `LocalTtyTerminalManager::`, and keeps the TUI driver plus terminal manager alive. - [`crates/warp_tui/src/terminal_session_view.rs`](https://github.com/warpdotdev/warp/blob/526ade4522df0e65f138c67dcbcb90f1a3ce63e9/crates/warp_tui/src/terminal_session_view.rs) — constructs the production AI stack for the TUI surface: `ActiveSession`, `TuiConversationSelection`, `BlocklistAIContextModel`, `BlocklistAIInputModel`, `BlocklistAIActionModel`, and `BlocklistAIController`. -- [`crates/warp_tui/src/transcript_view.rs`](https://github.com/warpdotdev/warp/blob/526ade4522df0e65f138c67dcbcb90f1a3ce63e9/crates/warp_tui/src/transcript_view.rs) — subscribes to `BlocklistAIHistoryModel`, creates `TuiAgentBlockView`s for visible exchanges, and appends them to `TerminalModel::BlockList` as `RichContentType::AIBlock`. +- [`crates/warp_tui/src/transcript_view.rs`](https://github.com/warpdotdev/warp/blob/526ade4522df0e65f138c67dcbcb90f1a3ce63e9/crates/warp_tui/src/transcript_view.rs) — subscribes to `BlocklistAIHistoryModel`, creates `TuiAIBlock`s for visible exchanges, and appends them to `TerminalModel::BlockList` as `RichContentType::AIBlock`. - [`crates/warp_tui/src/tui_block_list_viewport_source.rs`](https://github.com/warpdotdev/warp/blob/526ade4522df0e65f138c67dcbcb90f1a3ce63e9/crates/warp_tui/src/tui_block_list_viewport_source.rs) — walks the canonical block-list sum tree, measures dirty rich-content views, updates cached heights, and renders terminal blocks plus TUI agent blocks. -- [`crates/warp_tui/src/agent_block.rs`](https://github.com/warpdotdev/warp/blob/526ade4522df0e65f138c67dcbcb90f1a3ce63e9/crates/warp_tui/src/agent_block.rs) — currently derives `TuiAgentBlockSection`s from `output.text_from_agent_output()`, which only yields `AIAgentOutputMessageType::Text` and therefore drops action/tool-call messages. +- [`crates/warp_tui/src/agent_block.rs`](https://github.com/warpdotdev/warp/blob/526ade4522df0e65f138c67dcbcb90f1a3ce63e9/crates/warp_tui/src/agent_block.rs) — currently derives `TuiAIBlockSection`s from `output.text_from_agent_output()`, which only yields `AIAgentOutputMessageType::Text` and therefore drops action/tool-call messages. - [`app/src/ai/agent/mod.rs`](https://github.com/warpdotdev/warp/blob/526ade4522df0e65f138c67dcbcb90f1a3ce63e9/app/src/ai/agent/mod.rs) — defines `AIAgentOutput`, ordered `AIAgentOutputMessage`s, `AIAgentOutputMessageType::Action(AIAgentAction)`, and helper iterators such as `text_from_agent_output()` and `actions()`. - [`app/src/ai/blocklist/block/view_impl/output.rs`](https://github.com/warpdotdev/warp/blob/526ade4522df0e65f138c67dcbcb90f1a3ce63e9/app/src/ai/blocklist/block/view_impl/output.rs) — GUI AI block rendering loops over `output.messages` in order and delegates each message variant to the appropriate renderer. - [`app/src/ai/blocklist/controller.rs`](https://github.com/warpdotdev/warp/blob/526ade4522df0e65f138c67dcbcb90f1a3ce63e9/app/src/ai/blocklist/controller.rs) — when a response stream finishes, queues emitted actions through `BlocklistAIActionModel::queue_actions` and later sends completed action results back to the model. @@ -25,7 +25,7 @@ flowchart LR Actions --> Executor["BlocklistAIActionExecutor
shared tool execution"] Executor --> Result["AIAgentInput::ActionResult
follow-up request"] History --> Transcript["TuiTranscriptView
dirty rich content"] - Transcript --> Block["TuiAgentBlockView"] + Transcript --> Block["TuiAIBlock"] Block --> Stub["executed a tool call"] ``` The rendering path and the execution path share the same `AIAgentOutputMessageType::Action` source message but remain separate: @@ -33,15 +33,15 @@ The rendering path and the execution path share the same `AIAgentOutputMessageTy - Execution is driven by `BlocklistAIController` and `BlocklistAIActionModel`. - Completed action results flow back to the LLM as `AIAgentInput::ActionResult`; the first TUI UI does not render those results. ### Ordered message-to-section adapter -Replace `TuiAgentBlockView::sections`' output extraction with an ordered pass over `AIAgentOutput.messages`. The GUI AI block already follows this shape in `output.rs`: one ordered pass over messages, then variant-specific rendering. The TUI matches the ordering pattern without porting the GUI renderer. +Replace `TuiAIBlock::sections`' output extraction with an ordered pass over `AIAgentOutput.messages`. The GUI AI block already follows this shape in `output.rs`: one ordered pass over messages, then variant-specific rendering. The TUI matches the ordering pattern without porting the GUI renderer. ```mermaid flowchart LR Output["AIAgentOutput"] --> Messages["messages
server order"] - Messages --> Adapter["TuiAgentBlockView::sections"] - Adapter --> Input["Input(Vec)"] + Messages --> Adapter["TuiAIBlock::sections"] + Adapter --> Input["Input(String)"] Adapter --> Text["PlainText(String)"] Adapter --> Tool["ToolCall(Box)"] - Input --> Render["TuiAgentBlockSection::render_element"] + Input --> Render["TuiAIBlockSection::render_element"] Text --> Render Tool --> Render Render --> Label["TuiText
executed a tool call"] @@ -49,24 +49,24 @@ flowchart LR Use a flat section enum so the block stays extensible as more message types get TUI renderers: ```rust #[derive(Clone, Debug, Eq, PartialEq)] -enum TuiAgentBlockSection { - Input(Vec), +enum TuiAIBlockSection { + Input(String), PlainText(String), ToolCall(Box), } ``` The `ToolCall` variant carries the full `AIAgentAction` even though the first renderer ignores the fields. The section is derived render data, not durable UI state. Carrying the action now gives future renderers direct access to `id`, `task_id`, `requires_result`, and the concrete `AIAgentActionType` without changing the adapter API. `AIAgentAction` is large, so the variant is boxed to satisfy clippy's `large_enum_variant`. The adapter behavior should be: -- `AIAgentInput::display_query()` values become `TuiAgentBlockSection::Input`, split into one entry per line. -- `AIAgentOutputMessageType::Text(AIAgentText { sections })` becomes one `TuiAgentBlockSection::PlainText` per non-empty `AIAgentTextSection::PlainText`. -- `AIAgentOutputMessageType::Action(action)` becomes one `TuiAgentBlockSection::ToolCall(Box::new(action.clone()))`. +- Each input's `AIAgentInput::display_query()` value is joined with newlines into a single `TuiAIBlockSection::Input`; the renderer splits it per line, prefixing the first line with the prompt marker and indenting continuation lines beneath it. +- `AIAgentOutputMessageType::Text(AIAgentText { sections })` becomes one `TuiAIBlockSection::PlainText` per non-empty `AIAgentTextSection::PlainText`. +- `AIAgentOutputMessageType::Action(action)` becomes one `TuiAIBlockSection::ToolCall(Box::new(action.clone()))`. - Code, table, image, Mermaid, reasoning, summarization, todo, subagent, web, artifact, skill, message-bus, lifecycle-event, debug, and comments-addressed messages remain unsupported until the TUI has specific renderers for them. -The `ToolCall` arm of `TuiAgentBlockSection::render_element` renders exactly `executed a tool call`. The first implementation does not include the tool name, status, arguments, or result details. The stub is styled as a muted status row (`theme.terminal_colors().bright.black` plus `Modifier::DIM`) so it reads as a tool-call event rather than blending into the agent's plain-text prose. +The `ToolCall` arm of `TuiAIBlockSection::render_element` renders exactly `executed a tool call`. The first implementation does not include the tool name, status, arguments, or result details. The stub is styled as a muted status row (`theme.terminal_colors().bright.black` plus `Modifier::DIM`) so it reads as a tool-call event rather than blending into the agent's plain-text prose. ### State ownership -Do not store a materialized `Vec` on `TuiAgentBlockView`. Like the GUI AI block, durable state should be stored only when it represents interaction state that must survive renders. -`TuiAgentBlockView` holds the exchange identity and backing model; sections are re-derived on each render: +Do not store a materialized `Vec` on `TuiAIBlock`. Like the GUI AI block, durable state should be stored only when it represents interaction state that must survive renders. +`TuiAIBlock` holds the exchange identity and backing model; sections are re-derived on each render: ```rust -pub(super) struct TuiAgentBlockView { +pub(super) struct TuiAIBlock { conversation_id: AIConversationId, exchange_id: AIAgentExchangeId, model: Rc>, @@ -83,22 +83,22 @@ flowchart LR Event["UpdatedStreamingExchange"] --> Dirty["TuiTranscriptView::mark_exchange_dirty"] Dirty --> Mark["block_list_mut().mark_rich_content_dirty(view_id)"] Mark --> Source["TuiBlockListViewportSource::visible_items"] - Source --> Measure["TuiAgentBlockView::desired_height"] + Source --> Measure["TuiAIBlock::desired_height"] Measure --> Heights["update_rich_content_heights"] Source --> Render["render child view"] ``` When future TUI tool cards show live states like queued, blocked, running, failed, or cancelled, `TuiTranscriptView` should subscribe to `BlocklistAIActionModel` events and dirty the owning rich-content item by action ID. That coupling should not be added until visible status depends on it. ### Automatic execution policy TUI-created conversations should opt into `AIConversationAutoexecuteMode::RunToCompletion` through `TuiConversationSelection`, so emitted tool calls can execute without a first-pass TUI approval UI. -Add an explicit constructor: +`TuiConversationSelection::new` takes the new-conversation autoexecute mode explicitly: ```rust -pub(super) fn new_with_autoexecute_override( +pub(super) fn new( terminal_surface_id: EntityId, autoexecute_override: AIConversationAutoexecuteMode, ctx: &mut ModelContext>, ) -> Self ``` -Use this constructor from `TuiTerminalSessionView::new` with `AIConversationAutoexecuteMode::RunToCompletion`. Keep `toggle_pending_query_autoexecute` intact so a future TUI affordance can switch between `RunToCompletion` and `RespectUserSettings`. +Call it from `TuiTerminalSessionView::new` with `AIConversationAutoexecuteMode::RunToCompletion`. Callers pass the mode directly rather than deriving it from sandbox detection. Keep `toggle_pending_query_autoexecute` intact so a future TUI affordance can switch between `RunToCompletion` and `RespectUserSettings`. Do not duplicate permission logic in the TUI. `BlocklistAIPermissions`, execution profiles, and autonomous/sandboxed execution behavior remain inside the shared action execution path. ### Shell-command PTY bridge Shell-command tool calls require one TUI-specific bridge because the shared shell command executor emits model events rather than directly writing to the TUI PTY. @@ -118,7 +118,7 @@ Update `PtyIntentEvent` so: Subscribe `TuiTerminalSessionView` to `action_model.as_ref(ctx).shell_command_executor(ctx)` and translate: - `ShellCommandExecutorEvent::ExecuteCommand { action_id, command }` into `TuiTerminalSessionEvent::ExecuteCommand`. - `ShellCommandExecutorEvent::WriteToPty { input, mode }` into `TuiTerminalSessionEvent::WriteAgentInput`. -- `CancelExecution` and `TransferControlToUser { .. }` as no-ops for the first pass. +- `CancelExecution` and `TransferControlToUser { .. }` are no-ops for the first pass. Cancellation is left as a `TODO(tui-agent-cancel)`: the GUI cancel path sends an interrupt (ETX) to the running command's PTY because the user's Ctrl-C is routed to the agent block instead of the command's shell, and the TUI should mirror that once a control-handoff affordance exists. The command event should use `CommandExecutionSource::AI` and `AgentInteractionMetadata::new_hidden(action_id, conversation_id)`. The conversation ID should be looked up with `BlocklistAIHistoryModel::conversation_id_for_action(action_id, ctx.view_id())`, and the active session ID should come from the current terminal model active block. ### Headless file-edit execution (deferred) `RequestFileEdits` renders in the transcript like other tool calls, but it is not executed on non-GUI surfaces in this branch: `RequestFileEditsExecutor::execute` still requires a registered GUI `CodeDiffView` and returns `NotReady` otherwise (`app/src/ai/blocklist/action_model/execute/request_file_edits.rs`). @@ -141,9 +141,7 @@ Add unit tests in `crates/warp_tui/src/agent_block_tests.rs` for the ordered mes - Unsupported text sections and unsupported message variants are ignored without disturbing adjacent supported items. - `desired_height` accounts for the tool-call stub line. Keep transcript dirty/reflow tests focused on `UpdatedStreamingExchange`; no action-model event dirtying test is needed for the static stub. -Add shell bridge tests: -- `TuiTerminalSessionEvent::WriteAgentInput` maps to `PtyIntent::WriteAgentInput`. -- The `ExecuteCommand` → `PtyIntent::ExecuteCommand` branch and the shell-executor event translation are covered by compilation only, because constructing an `ExecuteCommandEvent` requires `SessionId`, which is intentionally not exported through `tui_export` for this feature. Add direct tests for these when a session-id test seam is available. +The shell-command PTY bridge is covered by compilation only for this branch. `TuiTerminalSessionEvent`'s `PtyIntentEvent` mapping and the shell-executor event translation are exercised through the type system rather than dedicated unit tests, since constructing an `ExecuteCommandEvent` requires `SessionId`, which is intentionally not exported through `tui_export` for this feature. Add direct tests when a session-id test seam is available. File-edit execution is out of scope for this branch (deferred to the stacked follow-up), so no `RequestFileEditsExecutor` persistence tests are added here. Run targeted tests first: ```bash From 6bbc7bc7778d1c592a2b66c3d9ccea41c832a24e Mon Sep 17 00:00:00 2001 From: harryalbert Date: Wed, 1 Jul 2026 16:35:17 -0400 Subject: [PATCH 5/6] extra testing --- crates/warp_tui/src/agent_block_tests.rs | 110 ++++++++++++++++++ crates/warp_tui/src/conversation_selection.rs | 21 ++-- .../src/conversation_selection_tests.rs | 18 +-- crates/warp_tui/src/terminal_session_view.rs | 19 ++- 4 files changed, 134 insertions(+), 34 deletions(-) diff --git a/crates/warp_tui/src/agent_block_tests.rs b/crates/warp_tui/src/agent_block_tests.rs index b88fb18899..05d811b0f5 100644 --- a/crates/warp_tui/src/agent_block_tests.rs +++ b/crates/warp_tui/src/agent_block_tests.rs @@ -189,6 +189,100 @@ fn agent_block_renders_tool_calls_in_message_order() { }); } +#[test] +fn agent_block_renders_multiple_tool_calls_in_order() { + App::test((), |app| async move { + app.add_singleton_model(|_| Appearance::mock()); + app.read(|app_ctx| { + let first = test_action("action-1"); + let second = test_action("action-2"); + let block = test_agent_block(FakeAgentBlockModel { + inputs: Vec::new(), + status: complete_output_messages(vec![ + action_message("message-1", first.clone()), + action_message("message-2", second.clone()), + ]), + }); + + assert_eq!( + block.sections(app_ctx), + vec![ + TuiAIBlockSection::ToolCall(Box::new(first)), + TuiAIBlockSection::ToolCall(Box::new(second)), + ] + ); + + let mut presenter = TuiPresenter::new(); + let frame = presenter.present_element( + block.render_element(app_ctx), + TuiRect::new(0, 0, 40, 3), + app_ctx, + ); + assert_eq!( + frame + .buffer + .to_lines() + .into_iter() + .map(|line| line.trim_end().to_owned()) + .collect::>(), + vec!["executed a tool call", "executed a tool call", ""], + ); + }); + }); +} + +#[test] +fn agent_block_desired_height_accounts_for_tool_call_stub() { + App::test((), |app| async move { + app.add_singleton_model(|_| Appearance::mock()); + app.read(|app_ctx| { + let block = test_agent_block(FakeAgentBlockModel { + inputs: Vec::new(), + status: complete_output_messages(vec![action_message( + "message-1", + test_action("action-1"), + )]), + }); + // One tool-call stub line plus the block's bottom padding row. + assert_eq!(block.desired_height(40, app_ctx), 2); + }); + }); +} + +#[test] +fn agent_block_ignores_unsupported_message_variants() { + App::test((), |app| async move { + app.read(|app_ctx| { + let block = test_agent_block(FakeAgentBlockModel { + inputs: Vec::new(), + status: complete_output_messages(vec![ + text_message( + "message-1", + vec![AIAgentTextSection::PlainText { + text: "before".to_owned().into(), + }], + ), + reasoning_message("message-2"), + text_message( + "message-3", + vec![AIAgentTextSection::PlainText { + text: "after".to_owned().into(), + }], + ), + ]), + }); + + assert_eq!( + block.sections(app_ctx), + vec![ + TuiAIBlockSection::PlainText("before".to_owned()), + TuiAIBlockSection::PlainText("after".to_owned()), + ] + ); + }); + }); +} + #[test] fn agent_block_omits_unsupported_sections_until_the_tui_can_render_them() { App::test((), |app| async move { @@ -301,6 +395,22 @@ fn action_message(id: &str, action: AIAgentAction) -> AIAgentOutputMessage { } } +/// Builds a reasoning output message (an unsupported variant for the TUI). +fn reasoning_message(id: &str) -> AIAgentOutputMessage { + AIAgentOutputMessage { + id: MessageId::new(id.to_owned()), + message: AIAgentOutputMessageType::Reasoning { + text: AIAgentText { + sections: vec![AIAgentTextSection::PlainText { + text: "thinking".to_owned().into(), + }], + }, + finished_duration: None, + }, + citations: Vec::new(), + } +} + /// Builds a tool-call action for message-ordering tests. fn test_action(id: &str) -> AIAgentAction { AIAgentAction { diff --git a/crates/warp_tui/src/conversation_selection.rs b/crates/warp_tui/src/conversation_selection.rs index 02f1181165..43f0165cd5 100644 --- a/crates/warp_tui/src/conversation_selection.rs +++ b/crates/warp_tui/src/conversation_selection.rs @@ -8,7 +8,6 @@ use warpui::{AppContext, EntityId, ModelContext, SingletonEntity}; /// TUI-owned next-prompt conversation selection. pub(super) struct TuiConversationSelection { terminal_surface_id: EntityId, - default_autoexecute_override: AIConversationAutoexecuteMode, pending_query_state: PendingQueryState, } @@ -16,7 +15,6 @@ impl TuiConversationSelection { /// Creates TUI conversation selection for a terminal surface. pub(super) fn new( terminal_surface_id: EntityId, - autoexecute_override: AIConversationAutoexecuteMode, ctx: &mut ModelContext>, ) -> Self { ctx.subscribe_to_model( @@ -27,21 +25,14 @@ impl TuiConversationSelection { // TODO: Implement actual permissions once settings are in place and there is a UI for permissions requests. // For now, we just always set fast-forward to on. let pending_query_state = PendingQueryState::New { - autoexecute_override, + autoexecute_override: AIConversationAutoexecuteMode::RunToCompletion, }; Self { terminal_surface_id, - default_autoexecute_override: autoexecute_override, pending_query_state, } } - /// Returns the configured new-conversation state for this TUI surface. - fn default_new_conversation_state(&self) -> PendingQueryState { - PendingQueryState::New { - autoexecute_override: self.default_autoexecute_override, - } - } /// Returns the selected existing conversation ID. fn selected_id(&self) -> Option { @@ -128,7 +119,15 @@ impl ConversationSelection for TuiConversationSelection { ctx: &mut ModelContext>, ) { let previous_conversation_id = self.selected_id(); - self.set_pending_query_state(self.default_new_conversation_state(), ctx); + // TODO: Implement actual permissions once settings are in place and there is a UI for permissions requests. + // For now, we just always set fast-forward to on. + self.set_pending_query_state( + PendingQueryState::New { + autoexecute_override: AIConversationAutoexecuteMode::RunToCompletion, + }, + ctx, + ); + if let Some(previous_conversation_id) = previous_conversation_id { Self::emit_deactivated(previous_conversation_id, false, ctx); } diff --git a/crates/warp_tui/src/conversation_selection_tests.rs b/crates/warp_tui/src/conversation_selection_tests.rs index f69d20b82a..c3da12ab84 100644 --- a/crates/warp_tui/src/conversation_selection_tests.rs +++ b/crates/warp_tui/src/conversation_selection_tests.rs @@ -1,6 +1,6 @@ use warp::tui_export::{ - AIConversationAutoexecuteMode, AIConversationId, AgentViewEntryOrigin, BlocklistAIHistoryEvent, - BlocklistAIHistoryModel, ConversationSelection, ConversationSelectionHandle, + AIConversationId, AgentViewEntryOrigin, BlocklistAIHistoryEvent, BlocklistAIHistoryModel, + ConversationSelection, ConversationSelectionHandle, }; use warp_core::execution_mode::{AppExecutionMode, ExecutionMode}; use warpui::{App, EntityId, ModelHandle}; @@ -18,11 +18,8 @@ fn build_tui_selection( let history = app.add_singleton_model(|_| BlocklistAIHistoryModel::default()); let terminal_surface_id = EntityId::new(); let selection = app.add_model(|ctx| { - Box::new(TuiConversationSelection::new( - terminal_surface_id, - AIConversationAutoexecuteMode::RespectUserSettings, - ctx, - )) as Box + Box::new(TuiConversationSelection::new(terminal_surface_id, ctx)) + as Box }); (history, selection, terminal_surface_id) } @@ -133,11 +130,8 @@ fn tui_new_conversation_preserves_pending_autoexecute_override() { let history = app.add_singleton_model(|_| BlocklistAIHistoryModel::default()); let terminal_surface_id = EntityId::new(); let selection = app.add_model(|ctx| { - Box::new(TuiConversationSelection::new( - terminal_surface_id, - AIConversationAutoexecuteMode::RunToCompletion, - ctx, - )) as Box + Box::new(TuiConversationSelection::new(terminal_surface_id, ctx)) + as Box }); let conversation_id = selection diff --git a/crates/warp_tui/src/terminal_session_view.rs b/crates/warp_tui/src/terminal_session_view.rs index 5685744068..dd8112c305 100644 --- a/crates/warp_tui/src/terminal_session_view.rs +++ b/crates/warp_tui/src/terminal_session_view.rs @@ -5,12 +5,12 @@ use std::sync::Arc; use parking_lot::FairMutex; use warp::editor::CodeEditorModel; use warp::tui_export::{ - AIAgentPtyWriteMode, AIConversationAutoexecuteMode, ActiveSession, AgentInteractionMetadata, - AgentViewEntryOrigin, Appearance, BlocklistAIActionModel, BlocklistAIContextModel, - BlocklistAIController, BlocklistAIHistoryModel, BlocklistAIInputModel, CommandExecutionSource, - ConversationSelection, ConversationSelectionHandle, ExecuteCommandEvent, - GetRelevantFilesController, ModelEvent, PtyIntent, PtyIntentEvent, ShellCommandExecutorEvent, - TerminalModel, TerminalSurface, TerminalSurfaceInit, + AIAgentPtyWriteMode, ActiveSession, AgentInteractionMetadata, AgentViewEntryOrigin, Appearance, + BlocklistAIActionModel, BlocklistAIContextModel, BlocklistAIController, + BlocklistAIHistoryModel, BlocklistAIInputModel, CommandExecutionSource, ConversationSelection, + ConversationSelectionHandle, ExecuteCommandEvent, GetRelevantFilesController, ModelEvent, + PtyIntent, PtyIntentEvent, ShellCommandExecutorEvent, TerminalModel, TerminalSurface, + TerminalSurfaceInit, }; use warp_core::ui::theme::Fill as ThemeFill; use warpui::SingletonEntity; @@ -74,11 +74,8 @@ impl TuiTerminalSessionView { let active_session = ctx.add_model(|ctx| ActiveSession::new(sessions.clone(), model_events.clone(), ctx)); let conversation_selection = ctx.add_model(|ctx| { - Box::new(TuiConversationSelection::new( - terminal_surface_id, - AIConversationAutoexecuteMode::RunToCompletion, - ctx, - )) as Box + Box::new(TuiConversationSelection::new(terminal_surface_id, ctx)) + as Box }); let context_model = ctx.add_model(|ctx| { BlocklistAIContextModel::new( From 44a433b3e86814e547da50a8f7daabbf385a427c Mon Sep 17 00:00:00 2001 From: harryalbert Date: Wed, 1 Jul 2026 21:33:31 -0400 Subject: [PATCH 6/6] use .finish --- crates/warp_tui/src/agent_block.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/crates/warp_tui/src/agent_block.rs b/crates/warp_tui/src/agent_block.rs index db05a97cde..b45a54bcf5 100644 --- a/crates/warp_tui/src/agent_block.rs +++ b/crates/warp_tui/src/agent_block.rs @@ -220,16 +220,15 @@ impl TuiAIBlockSection { // TODO: add richer rendering for each tool call type. This is just a rendering stub to build off of. let text_color = Fill::from(ThemeFill::from(theme.terminal_colors().bright.black)).into(); - Box::new( - TuiContainer::new( - TuiText::new("executed a tool call").with_style( - TuiStyle::default() - .fg(text_color) - .add_modifier(Modifier::DIM), - ), - ) - .with_padding_top(top_padding), + TuiContainer::new( + TuiText::new("executed a tool call").with_style( + TuiStyle::default() + .fg(text_color) + .add_modifier(Modifier::DIM), + ), ) + .with_padding_top(top_padding) + .finish() } } }