diff --git a/docs/keybindings.md b/docs/keybindings.md index eb588d45..69902423 100644 --- a/docs/keybindings.md +++ b/docs/keybindings.md @@ -92,6 +92,8 @@ These bindings are active when Coven Code is displaying a yes/no confirmation pr | `Enter` | defaultAction | Accept the highlighted default option | | `Escape` | cancel | Cancel the prompt and deny the action | +In the Next renderer (`COVEN_CODE_UI=next`), a tool permission request opens with **No, deny** highlighted, so `Enter` on a new request denies. Keys other than `Escape` are ignored until 400 ms pass with no key pressed. Each ignored key restarts that wait, so typing meant for the prompt or a held key cannot approve a request. `Escape` always denies immediately. + --- ### Inspector Context (Next renderer) diff --git a/src-rust/crates/acp/src/diagnostic_tests.rs b/src-rust/crates/acp/src/diagnostic_tests.rs index 3496b6d0..e7ab376a 100644 --- a/src-rust/crates/acp/src/diagnostic_tests.rs +++ b/src-rust/crates/acp/src/diagnostic_tests.rs @@ -204,6 +204,7 @@ async fn permission_reply(reply: Value) -> PermissionDecision { let (tx, rx) = tokio::sync::oneshot::channel(); let pending = PendingPermissionRequest { tool_use_id: "fixture-tool".into(), + call_id: None, reason: "fixture approval".into(), decision_tx: Some(tx), request: PermissionRequest { @@ -309,6 +310,7 @@ fn permission_io_error_is_denied_without_logging_error_contents() { let (tx, rx) = tokio::sync::oneshot::channel(); let pending = PendingPermissionRequest { tool_use_id: "fixture-tool".into(), + call_id: None, reason: "fixture approval".into(), decision_tx: Some(tx), request: PermissionRequest { diff --git a/src-rust/crates/acp/src/permission.rs b/src-rust/crates/acp/src/permission.rs index 3b54181b..6ce95c4c 100644 --- a/src-rust/crates/acp/src/permission.rs +++ b/src-rust/crates/acp/src/permission.rs @@ -47,6 +47,7 @@ pub async fn forward_pending( ) { let PendingPermissionRequest { tool_use_id, + call_id: _, request, reason, decision_tx, @@ -345,6 +346,7 @@ mod tests { let (tx, rx) = tokio::sync::oneshot::channel(); store.lock().queue.push_back(PendingPermissionRequest { tool_use_id: id.into(), + call_id: None, request: permission_request("Bash", "fixture", None, None, false), reason: "fixture".into(), decision_tx: Some(tx), diff --git a/src-rust/crates/cli/src/main.rs b/src-rust/crates/cli/src/main.rs index 1ac63e5c..d33cf9c5 100644 --- a/src-rust/crates/cli/src/main.rs +++ b/src-rust/crates/cli/src/main.rs @@ -2035,16 +2035,14 @@ fn handle_bridge_permission_response( pending_permissions: &mut claurst_tools::PendingPermissionStore, tool_use_id: &str, response: claurst_bridge::PermissionResponseKind, -) { +) -> Option<(String, bool)> { if !dialog .as_ref() .is_some_and(|pr| pr.tool_use_id == tool_use_id) { - return; + return None; } - let Some(mut pending) = pending_permissions.waiting.remove(tool_use_id) else { - return; - }; + let mut pending = pending_permissions.waiting.remove(tool_use_id)?; use claurst_bridge::PermissionResponseKind; use claurst_core::permissions::PermissionDecision; @@ -2055,10 +2053,13 @@ fn handle_bridge_permission_response( } PermissionResponseKind::Deny => PermissionDecision::Deny, }; + let allowed = matches!(decision, PermissionDecision::Allow); if let Some(tx) = pending.decision_tx.take() { let _ = tx.send(decision); } *dialog = None; + // The decision belongs to the model call that asked, when known. + Some((pending.call_id.unwrap_or(pending.tool_use_id), allowed)) } fn permission_request_from_core( @@ -4067,6 +4068,12 @@ async fn run_interactive( } _ => claurst_core::permissions::PermissionDecision::Allow, }; + // Shown beside the tool's execution state; an + // allowed call is not a successful one. + app.record_permission_decision( + pending.call_id.as_deref().unwrap_or(&tool_use_id), + selected_key != Some('n'), + ); if let Some(manager) = tool_ctx.permission_manager.as_ref() { if let Ok(mut manager) = manager.lock() { @@ -4221,7 +4228,16 @@ async fn run_interactive( match reevaluated { Some(claurst_core::permissions::PermissionDecision::Ask { .. }) | None => { let tool_use_id = pending.tool_use_id.clone(); - app.permission_request = Some(permission_request_from_core(&pending)); + let request = permission_request_from_core(&pending); + // Next guards approvals against stray keys (#411); + // Classic keeps its existing dialog behavior. + app.permission_request = Some( + if app.renderer == claurst_tui::application::UiRenderer::Next { + request.guarded() + } else { + request + }, + ); pending_permissions .lock() .waiting @@ -4514,6 +4530,7 @@ async fn run_interactive( && app.renderer == claurst_tui::application::UiRenderer::Next { app.note_cancellation_sent(); + app.interrupt_tool_rows(); } app.is_streaming = false; app.status_message = Some("Cancelled by remote control.".to_string()); @@ -4522,12 +4539,15 @@ async fn run_interactive( tool_use_id, response, }) => { - handle_bridge_permission_response( + let decided = handle_bridge_permission_response( &mut app.permission_request, &mut pending_permissions.lock(), &tool_use_id, response, ); + if let Some((call_id, allowed)) = decided { + app.record_permission_decision(&call_id, allowed); + } } Ok(TuiBridgeEvent::SessionNameUpdate { title }) => { session.title = Some(title.clone()); @@ -6125,6 +6145,7 @@ mod tests { ( claurst_tools::PendingPermissionRequest { tool_use_id: id.into(), + call_id: None, request: claurst_core::permissions::PermissionRequest { tool_name: "Write".into(), description: "Write a file".into(), @@ -6165,6 +6186,28 @@ mod tests { } } + #[test] + fn bridge_permission_decisions_belong_to_the_model_call_that_asked() { + use claurst_bridge::PermissionResponseKind as Response; + + for (call_id, response, expected) in [ + (Some("toolu_model"), Response::Allow, ("toolu_model", true)), + (Some("toolu_model"), Response::Deny, ("toolu_model", false)), + (None, Response::AllowSession, ("perm-1", true)), + ] { + let (mut pending, _rx) = bridge_permission_fixture("perm-1"); + pending.call_id = call_id.map(str::to_string); + let mut dialog = Some(permission_request_from_core(&pending)); + let mut store = claurst_tools::PendingPermissionStore::default(); + store.waiting.insert("perm-1".into(), pending); + + let decided = + handle_bridge_permission_response(&mut dialog, &mut store, "perm-1", response); + + assert_eq!(decided, Some((expected.0.to_string(), expected.1))); + } + } + #[test] fn bridge_permission_response_ignores_other_and_repeated_ids() { use claurst_bridge::PermissionResponseKind as Response; diff --git a/src-rust/crates/query/src/lib.rs b/src-rust/crates/query/src/lib.rs index 4ff12b87..8facef74 100644 --- a/src-rust/crates/query/src/lib.rs +++ b/src-rust/crates/query/src/lib.rs @@ -3961,9 +3961,10 @@ async fn execute_tool_blocks_with_hooks( } else { let name = p.name.clone(); let input = p.input.clone(); - futures::future::Either::Right(async move { + let call_id = p.id.clone(); + futures::future::Either::Right(claurst_tools::with_tool_call(call_id, async move { execute_tool(&name, &input, tools, tool_ctx).await - }) + })) } }) .collect(); diff --git a/src-rust/crates/tools/src/lib.rs b/src-rust/crates/tools/src/lib.rs index 1c0c86bb..ad6d8c3d 100644 --- a/src-rust/crates/tools/src/lib.rs +++ b/src-rust/crates/tools/src/lib.rs @@ -174,11 +174,29 @@ pub enum PermissionLevel { #[derive(Debug)] pub struct PendingPermissionRequest { pub tool_use_id: String, + /// The model's tool-use id for the call that asked, when known. Distinct + /// from `tool_use_id`, which keys this request and may be synthetic. + pub call_id: Option, pub request: claurst_core::permissions::PermissionRequest, pub reason: String, pub decision_tx: Option>, } +tokio::task_local! { + static CURRENT_TOOL_CALL: String; +} + +/// Run `future` as the execution of the model tool call `call_id`, so a +/// permission request it raises can be attributed to that call. +pub async fn with_tool_call(call_id: String, future: F) -> F::Output { + CURRENT_TOOL_CALL.scope(call_id, future).await +} + +/// The model tool call being executed by the current task, if any. +pub fn current_tool_call() -> Option { + CURRENT_TOOL_CALL.try_with(Clone::clone).ok() +} + #[derive(Default)] pub struct PendingPermissionStore { pub queue: VecDeque, @@ -379,6 +397,7 @@ impl ToolContext { self.session_id, self.current_turn.fetch_add(1, Ordering::Relaxed) ), + call_id: current_tool_call(), request, reason: interactive_reason.unwrap_or(reason), decision_tx: Some(tx), @@ -799,6 +818,23 @@ pub fn find_tool(name: &str) -> Option> { #[cfg(test)] mod tests { + #[tokio::test] + async fn each_concurrent_tool_call_sees_its_own_call_id() { + assert_eq!(super::current_tool_call(), None); + let probe = || async { + tokio::task::yield_now().await; + super::current_tool_call() + }; + let (first, second) = futures::future::join( + super::with_tool_call("toolu_a".to_string(), probe()), + super::with_tool_call("toolu_b".to_string(), probe()), + ) + .await; + assert_eq!(first.as_deref(), Some("toolu_a")); + assert_eq!(second.as_deref(), Some("toolu_b")); + assert_eq!(super::current_tool_call(), None); + } + use super::*; struct AskPermissionHandler { diff --git a/src-rust/crates/tui/src/app.rs b/src-rust/crates/tui/src/app.rs index 0beff54a..2aa8a452 100644 --- a/src-rust/crates/tui/src/app.rs +++ b/src-rust/crates/tui/src/app.rs @@ -675,6 +675,11 @@ pub struct App { pub next_shell: crate::next::ShellState, /// A user request to cancel the running turn, until the runtime settles it. pub cancellation: crate::application::TurnCancellation, + /// Permission decisions by tool use id (true = allowed). + pub permission_decisions: std::collections::HashMap, + /// Tool rows of a cancelled turn, kept as evidence until the next user + /// turn; the live list is cleared on cancel. + pub interrupted_tools: Vec, // CLI control-key routing and the normal UI handler resolve each event once. resolved_keybinding: Option<(KeyEvent, bool, KeybindingResult)>, @@ -1261,6 +1266,8 @@ impl App { renderer: crate::application::UiRenderer::default(), next_shell: crate::next::ShellState::default(), cancellation: crate::application::TurnCancellation::default(), + permission_decisions: std::collections::HashMap::new(), + interrupted_tools: Vec::new(), resolved_keybinding: None, cursor_pos: 0, auto_scroll: true, @@ -1665,6 +1672,10 @@ impl App { } fn begin_user_turn_snapshot(&mut self) { + // A new turn replaces the previous turn's interruption evidence, and + // decisions are kept only while their tool row is still shown. + self.interrupted_tools.clear(); + self.prune_permission_decisions(); self.turn_metadata.push(TurnMetadata { submitted_at: Some(format_turn_time_label()), model_name: Some(self.model_name.clone()), @@ -2304,6 +2315,8 @@ impl App { self.streaming_text.clear(); self.streaming_thinking.clear(); self.tool_use_blocks.clear(); + self.interrupted_tools.clear(); + self.permission_decisions.clear(); self.turn_metadata.clear(); self.cost_usd = 0.0; self.invalidate_transcript(); @@ -2577,8 +2590,20 @@ impl App { /// (overage / voice / memory), which render as overlays but let the user /// keep typing underneath. pub fn any_blocking_modal_open(&self) -> bool { + self.permission_request.is_some() || self.blocking_surface_open_besides_permission() + } + + /// Like [`Self::any_modal_open`] but ignoring a pending permission + /// request, for renderers that draw the request themselves. + pub fn modal_open_besides_permission(&self) -> bool { + self.blocking_surface_open_besides_permission() + || self.overage_upsell.visible + || self.voice_mode_notice.visible + || self.memory_update_notification.visible + } + + fn blocking_surface_open_besides_permission(&self) -> bool { self.response_reader.visible - || self.permission_request.is_some() || self.rate_limit_recovery.visible || self.rewind_flow.visible || self.tasks_overlay.visible @@ -2869,6 +2894,10 @@ impl App { pub fn replace_messages(&mut self, messages: Vec) { self.messages = messages; + // Resume, rewind and clear replace the conversation; interruption + // evidence from the old one no longer describes what is shown. + self.interrupted_tools.clear(); + self.prune_permission_decisions(); let reader_message_gone = self.response_reader.message_index.is_some_and(|index| { self.messages @@ -3178,6 +3207,39 @@ impl App { true } + /// Remember the user's decision for a tool call so presentation can + /// show authorization separately from the execution result. + pub fn record_permission_decision(&mut self, tool_use_id: &str, allowed: bool) { + self.permission_decisions + .insert(tool_use_id.to_string(), allowed); + } + + fn prune_permission_decisions(&mut self) { + let shown: std::collections::HashSet<&str> = self + .tool_use_blocks + .iter() + .chain(&self.interrupted_tools) + .map(|block| block.id.as_str()) + .collect(); + self.permission_decisions + .retain(|id, _| shown.contains(id.as_str())); + } + + /// Clear the live tool rows of a turn being cancelled, keeping the ones + /// still running as evidence of what the cancel interrupted. + pub fn interrupt_tool_rows(&mut self) { + if self.renderer == crate::application::UiRenderer::Classic { + // Classic clears its tool rows on cancel, as it always has. + self.tool_use_blocks.clear(); + return; + } + let running = self + .tool_use_blocks + .drain(..) + .filter(|block| block.status == ToolStatus::Running); + self.interrupted_tools.extend(running); + } + /// Record a cancellation the loop performed without a key request /// (remote control), so it is shown pending until the worker stops. pub fn note_cancellation_sent(&mut self) { @@ -5166,7 +5228,7 @@ impl App { self.spinner_verb = None; self.streaming_text.clear(); self.streaming_thinking.clear(); - self.tool_use_blocks.clear(); + self.interrupt_tool_rows(); self.status_message = Some("Cancelled.".to_string()); self.request_turn_cancellation(); self.complete_current_turn_snapshot(true); @@ -5199,7 +5261,7 @@ impl App { self.spinner_verb = None; self.streaming_text.clear(); self.streaming_thinking.clear(); - self.tool_use_blocks.clear(); + self.interrupt_tool_rows(); self.status_message = Some("Cancelled.".to_string()); self.request_turn_cancellation(); self.complete_current_turn_snapshot(true); @@ -5990,7 +6052,7 @@ impl App { self.spinner_verb = None; self.streaming_text.clear(); self.streaming_thinking.clear(); - self.tool_use_blocks.clear(); + self.interrupt_tool_rows(); self.status_message = Some("Cancelled.".to_string()); self.request_turn_cancellation(); } else { @@ -7350,6 +7412,16 @@ impl App { .collect::>() .join("\n"), }; + // A cancelled call's closing result belongs to its interrupted + // row; it must not reappear as a second, failed card. + if let Some(row) = self + .interrupted_tools + .iter_mut() + .find(|row| row.id == tool_use_id) + { + row.output_preview = Some(Self::tool_result_preview(&result)); + continue; + } let existing = self .tool_use_blocks .iter() diff --git a/src-rust/crates/tui/src/application/view.rs b/src-rust/crates/tui/src/application/view.rs index 56b9b995..f73e3cae 100644 --- a/src-rust/crates/tui/src/application/view.rs +++ b/src-rust/crates/tui/src/application/view.rs @@ -5,6 +5,11 @@ //! transcript entries, live output, tool activity, an outstanding permission //! request and the composer draft. It holds copies for display only; it is //! never a source of authority for permissions, tools or persistence. +//! +//! Every string that came from a model, tool or user passes through +//! [`terminal_safe`], so no projection can carry a terminal control sequence +//! (clipboard writes, hyperlinks, screen clears, bidi overrides) to a +//! renderer. use crate::app::{App, ToolStatus}; use claurst_core::types::{ContentBlock, MessageContent, Role}; @@ -29,6 +34,11 @@ pub enum ToolActivityState { Running, Done, Error, + /// Running when the user cancelled; the runtime has not settled yet. + CancelRequested, + /// Interrupted by a cancel the runtime has settled. Whether the tool's + /// own effects stopped is not known here (see #373). + Cancelled, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -36,6 +46,70 @@ pub struct ToolActivity { pub id: String, pub name: String, pub state: ToolActivityState, + /// One-line, terminal-safe summary of what the call targets. + pub target: Option, + /// Terminal-safe arguments as sent by the model. + pub input: String, + /// Terminal-safe output preview, once the tool has produced one. + pub output: Option, + /// The user's permission decision for this call, when one was asked. + pub allowed: Option, +} + +/// Longest target shown on a tool row. +const TARGET_MAX_CHARS: usize = 60; + +/// Replace control characters with visible stand-ins: C0 controls and DEL +/// in caret notation (`^[` for Escape), C1 controls and bidirectional +/// overrides as U+FFFD. Newlines are kept and tabs become spaces. +pub fn terminal_safe(text: &str) -> String { + let mut safe = String::with_capacity(text.len()); + for ch in text.chars() { + match ch { + '\n' => safe.push('\n'), + '\t' => safe.push_str(" "), + '\u{7f}' => safe.push_str("^?"), + '\u{0}'..='\u{1f}' => { + safe.push('^'); + safe.push(char::from(b'@' + ch as u8)); + } + '\u{80}'..='\u{9f}' | '\u{202a}'..='\u{202e}' | '\u{2066}'..='\u{2069}' => { + safe.push('\u{fffd}') + } + _ => safe.push(ch), + } + } + safe +} + +/// The one-line target of a tool call: the first line of a command, or a +/// path, pattern, URL or query, shortened to [`TARGET_MAX_CHARS`]. +fn tool_target(input_json: &str, project_root: Option<&std::path::Path>) -> Option { + let input: serde_json::Map = + serde_json::from_str(input_json).ok()?; + let raw = [ + "command", + "file_path", + "path", + "pattern", + "url", + "query", + "description", + ] + .iter() + .find_map(|key| input.get(*key).and_then(serde_json::Value::as_str))?; + let first_line = raw.lines().next().unwrap_or_default(); + let relative = project_root + .and_then(|root| std::path::Path::new(first_line).strip_prefix(root).ok()) + .map(|path| path.display().to_string()) + .unwrap_or_else(|| first_line.to_string()); + let safe = terminal_safe(&relative); + if safe.chars().count() > TARGET_MAX_CHARS { + let short: String = safe.chars().take(TARGET_MAX_CHARS - 1).collect(); + Some(format!("{short}…")) + } else { + Some(safe) + } } #[derive(Debug, Clone, PartialEq, Eq)] @@ -43,6 +117,8 @@ pub struct PendingPermission { pub tool_use_id: String, pub tool_name: String, pub description: String, + /// Why the request is risky, when the producer explained it. + pub danger: String, pub input_preview: Option, /// Option labels in display order, each with its key. pub options: Vec<(char, String)>, @@ -88,7 +164,7 @@ impl TranscriptView { .messages .iter() .filter_map(|message| { - let text = message.get_all_text(); + let text = terminal_safe(&message.get_all_text()); let attachments: Vec = match &message.content { MessageContent::Blocks(blocks) => { blocks.iter().filter_map(attachment_label).collect() @@ -109,17 +185,36 @@ impl TranscriptView { }) }) .collect(); + let project_root = app.config.project_dir.as_deref(); + let cancelling = app.cancellation != crate::application::TurnCancellation::Idle; + let interrupted = app.interrupted_tools.iter().map(|block| { + let state = if cancelling { + ToolActivityState::CancelRequested + } else { + ToolActivityState::Cancelled + }; + (block, state) + }); let tools = app .tool_use_blocks .iter() - .map(|block| ToolActivity { - id: block.id.clone(), - name: block.name.clone(), - state: match block.status { + .map(|block| { + let state = match block.status { ToolStatus::Running => ToolActivityState::Running, ToolStatus::Done => ToolActivityState::Done, ToolStatus::Error => ToolActivityState::Error, - }, + }; + (block, state) + }) + .chain(interrupted) + .map(|(block, state)| ToolActivity { + id: block.id.clone(), + name: terminal_safe(&block.name), + state, + target: tool_target(&block.input_json, project_root), + input: terminal_safe(&block.input_json), + output: block.output_preview.as_deref().map(terminal_safe), + allowed: app.permission_decisions.get(&block.id).copied(), }) .collect(); let permission = app @@ -127,25 +222,31 @@ impl TranscriptView { .as_ref() .map(|request| PendingPermission { tool_use_id: request.tool_use_id.clone(), - tool_name: request.tool_name.clone(), - description: request.description.clone(), - input_preview: request.input_preview.clone(), + tool_name: terminal_safe(&request.tool_name), + description: terminal_safe(&request.description), + danger: terminal_safe(&request.danger_explanation), + input_preview: request.input_preview.as_deref().map(terminal_safe), options: request .options .iter() - .map(|option| (option.key, option.label.clone())) + .map(|option| (option.key, terminal_safe(&option.label))) .collect(), selected_option: request.selected_option, }); Self { entries, - live_text: app.streaming_text.clone(), + live_text: terminal_safe(&app.streaming_text), is_streaming: app.is_streaming, tools, permission, - status: app.status_message.clone(), - draft: app.prompt_input.text.clone(), - draft_cursor: app.prompt_input.cursor, + status: app.status_message.as_deref().map(terminal_safe), + draft: terminal_safe(&app.prompt_input.text), + // The cursor moves with the stand-ins drawn before it. + draft_cursor: app + .prompt_input + .text + .get(..app.prompt_input.cursor) + .map_or(0, |before| terminal_safe(before).len()), draft_attachments: app .prompt_input .pending_images diff --git a/src-rust/crates/tui/src/dialogs.rs b/src-rust/crates/tui/src/dialogs.rs index 6ff40027..d172fbdb 100644 --- a/src-rust/crates/tui/src/dialogs.rs +++ b/src-rust/crates/tui/src/dialogs.rs @@ -7,6 +7,7 @@ use ratatui::style::{Color, Modifier, Style}; use ratatui::text::{Line, Span}; use ratatui::widgets::{Block, Borders, Clear, Paragraph, Widget, Wrap}; use ratatui::Frame; +use std::time::{Duration, Instant}; // --------------------------------------------------------------------------- // Permission dialog kinds @@ -63,16 +64,46 @@ pub struct PermissionRequest { pub kind: PermissionDialogKind, pub options: Vec, pub selected_option: usize, + /// Whether the request is [`guarded`](Self::guarded). + pub arming: bool, + /// When a guarded request was opened, or the last key it ignored + /// arrived. Keys other than Escape count only after + /// [`PERMISSION_ARM_DELAY`] of quiet, so typing meant for the composer, + /// or a held key, cannot decide it. + pub quiet_since: Instant, } +/// How long a new permission request ignores decision keys. +pub const PERMISSION_ARM_DELAY: Duration = Duration::from_millis(400); + impl PermissionRequest { + /// A new request, with Classic's defaults: the first option focused + /// and keys accepted at once. + fn opened(self) -> Self { + self + } + + /// Guard the request against stray keys (Next renderer, #411): focus + /// the deny option, so the default action is never destructive, and + /// ignore keys other than Escape until a quiet gap has passed. + pub fn guarded(mut self) -> Self { + self.selected_option = self + .options + .iter() + .position(|option| option.key == 'n') + .unwrap_or_else(|| self.options.len().saturating_sub(1)); + self.arming = true; + self.quiet_since = Instant::now(); + self + } + /// Create a standard four-option dialog matching the TS dialog options: /// `y` — Yes, allow once /// `Y` — Yes, allow this session /// `p` — Yes, always allow (persistent) /// `n` — No, deny pub fn standard(tool_use_id: String, tool_name: String, description: String) -> Self { - Self { + Self::opened(Self { tool_use_id, tool_name, description: description.clone(), @@ -80,8 +111,10 @@ impl PermissionRequest { input_preview: None, kind: PermissionDialogKind::Generic, selected_option: 0, + arming: false, + quiet_since: Instant::now(), options: Self::default_options(), - } + }) } /// Build with a richer description derived from the full permission reason @@ -98,7 +131,7 @@ impl PermissionRequest { ) -> Self { let (description, danger_explanation) = split_reason(reason); - Self { + Self::opened(Self { tool_use_id, tool_name, description, @@ -106,8 +139,10 @@ impl PermissionRequest { input_preview, kind: PermissionDialogKind::Generic, selected_option: 0, + arming: false, + quiet_since: Instant::now(), options: Self::default_options(), - } + }) } /// Build a Bash-specific dialog, computing the options set based on whether @@ -125,7 +160,7 @@ impl PermissionRequest { suggested_prefix, }; - Self { + Self::opened(Self { tool_use_id, tool_name, description: String::new(), @@ -133,8 +168,10 @@ impl PermissionRequest { input_preview: Some(command), kind, selected_option: 0, + arming: false, + quiet_since: Instant::now(), options, - } + }) } pub fn powershell( @@ -143,7 +180,7 @@ impl PermissionRequest { reason: String, command: String, ) -> Self { - Self { + Self::opened(Self { tool_use_id, tool_name, description: String::new(), @@ -151,8 +188,10 @@ impl PermissionRequest { input_preview: Some(command.clone()), kind: PermissionDialogKind::PowerShell { command }, selected_option: 0, + arming: false, + quiet_since: Instant::now(), options: Self::default_options(), - } + }) } /// Build a FileRead-specific dialog (3 options: once / session / deny). @@ -166,7 +205,7 @@ impl PermissionRequest { let preview = path.clone(); let kind = PermissionDialogKind::FileRead { path }; - Self { + Self::opened(Self { tool_use_id, tool_name, description, @@ -174,8 +213,10 @@ impl PermissionRequest { input_preview: Some(preview), kind, selected_option: 0, + arming: false, + quiet_since: Instant::now(), options: Self::file_read_options(), - } + }) } /// Build a FileWrite-specific dialog (4 options: once / session / project / deny). @@ -194,7 +235,7 @@ impl PermissionRequest { let preview = path.clone(); let kind = PermissionDialogKind::FileWrite { path }; - Self { + Self::opened(Self { tool_use_id, tool_name, description, @@ -202,8 +243,10 @@ impl PermissionRequest { input_preview: Some(preview), kind, selected_option: 0, + arming: false, + quiet_since: Instant::now(), options: Self::file_write_options(), - } + }) } // ------------------------------------------------------------------ @@ -666,7 +709,18 @@ pub fn render_permission_dialog(frame: &mut Frame, pr: &PermissionRequest, area: /// - 4-option dialog (Generic / FileWrite / Bash without prefix): digits 1–4 valid. /// - 5-option dialog (Bash with prefix): digits 1–5 valid. pub fn handle_permission_key(pr: &mut PermissionRequest, key: KeyEvent) -> bool { + handle_permission_key_at(pr, key, Instant::now()) +} + +/// [`handle_permission_key`] at an explicit time. Before the request is +/// armed only Escape (deny) is accepted. +pub fn handle_permission_key_at(pr: &mut PermissionRequest, key: KeyEvent, now: Instant) -> bool { let option_count = pr.options.len(); + let armed = !pr.arming || now.saturating_duration_since(pr.quiet_since) >= PERMISSION_ARM_DELAY; + if !armed && key.code != KeyCode::Esc { + pr.quiet_since = now; + return false; + } match key.code { KeyCode::Char(c) => { if let Some(digit) = c.to_digit(10) { @@ -1584,6 +1638,8 @@ mod tests { fn permission_key_digit_selects_and_confirms() { let mut pr = PermissionRequest::standard("id".to_string(), "Bash".to_string(), "desc".to_string()); + // Old enough that decision keys count. + pr.quiet_since -= PERMISSION_ARM_DELAY; // Press '1' → selects option 0 (allow once) and confirms. let confirmed = handle_permission_key(&mut pr, key(KeyCode::Char('1'))); assert!(confirmed); @@ -1616,6 +1672,8 @@ mod tests { "git push".to_string(), Some("git ".to_string()), ); + // Old enough that decision keys count. + pr.quiet_since -= PERMISSION_ARM_DELAY; assert_eq!(pr.options.len(), 5); // '5' should select the 5th option (deny) and confirm. let confirmed = handle_permission_key(&mut pr, key(KeyCode::Char('5'))); @@ -1627,6 +1685,8 @@ mod tests { fn permission_key_char_shortcut_confirms() { let mut pr = PermissionRequest::standard("id".to_string(), "Bash".to_string(), "desc".to_string()); + // Old enough that decision keys count. + pr.quiet_since -= PERMISSION_ARM_DELAY; // Press 'n' → deny (index 3). let confirmed = handle_permission_key(&mut pr, key(KeyCode::Char('n'))); assert!(confirmed); @@ -1647,6 +1707,8 @@ mod tests { fn permission_key_up_down_navigation() { let mut pr = PermissionRequest::standard("id".to_string(), "Bash".to_string(), "desc".to_string()); + // Old enough that decision keys count. + pr.quiet_since -= PERMISSION_ARM_DELAY; pr.selected_option = 1; // Down. handle_permission_key(&mut pr, key(KeyCode::Down)); diff --git a/src-rust/crates/tui/src/next/mod.rs b/src-rust/crates/tui/src/next/mod.rs index f6713a8d..d96bcec6 100644 --- a/src-rust/crates/tui/src/next/mod.rs +++ b/src-rust/crates/tui/src/next/mod.rs @@ -19,7 +19,7 @@ pub mod primitives; pub mod style; use crate::app::App; -use crate::application::view::{EntryRole, ToolActivityState, TranscriptView}; +use crate::application::view::{EntryRole, ToolActivity, ToolActivityState, TranscriptView}; use crate::application::TurnCancellation; use claurst_core::appearance::Density; use claurst_core::keybindings::{KeyContext, KeybindingResolver, UserKeybindings}; @@ -118,7 +118,9 @@ impl NextPresentation { /// which are checked here without changing the list Classic's Enter gating /// relies on. pub fn delegates_to_classic(app: &App) -> bool { - app.any_modal_open() + // Next draws a pending permission request itself (#411) unless another + // surface is open too, in which case Classic's precedence applies. + app.modal_open_besides_permission() || app.familiar_switcher_open || app.effort_picker.visible || app.notifications.current_is_error() @@ -328,9 +330,12 @@ pub fn render_next(frame: &mut Frame, app: &App) { ) })); let hint_rows = u16::from(presentation.hints.is_some()); + // A pending approval takes the whole main pane at every size; the + // inspector keeps its state and returns once the request is decided. + let approval = view.permission.as_ref(); let layout = layout::plan( area, - app.next_shell.inspector.is_some(), + app.next_shell.inspector.is_some() && approval.is_none(), hint_rows, u16::try_from(composer.len()).unwrap_or(u16::MAX), ); @@ -339,7 +344,12 @@ pub fn render_next(frame: &mut Frame, app: &App) { Paragraph::new(workspace_line(appearance, app, area.width)), layout.header, ); - match layout.transcript { + if let (Some(request), Some(main)) = (approval, layout.transcript) { + let panel = approval_lines(appearance, app, request); + let lines = fit_approval(appearance, panel, width, usize::from(main.height)); + frame.render_widget(Paragraph::new(lines), main); + } + match layout.transcript.filter(|_| approval.is_none()) { Some(transcript) => { app.last_msg_area.set(transcript); app.last_selectable_area.set(transcript); @@ -519,12 +529,11 @@ fn transcript_lines(presentation: &NextPresentation, view: &TranscriptView) -> V lines.push(Line::default()); } } - for tool in &view.tools { - let (status, word) = tool_status(tool.state); - lines.push(primitives::tool_row( - appearance, &tool.name, status, word, false, - )); - } + lines.extend( + view.tools + .iter() + .map(|tool| tool_line(appearance, tool, false)), + ); if !view.live_text.is_empty() { lines.push(primitives::transcript_label(appearance, "coven")); lines.extend(view.live_text.lines().map(text)); @@ -538,6 +547,239 @@ fn tool_status(state: ToolActivityState) -> (Status, &'static str) { ToolActivityState::Running => (Status::Pending, "running"), ToolActivityState::Done => (Status::Evidence, "done"), ToolActivityState::Error => (Status::Blocked, "error"), + ToolActivityState::CancelRequested => (Status::Pending, "cancel requested"), + ToolActivityState::Cancelled => (Status::Blocked, "cancelled"), + } +} + +/// One tool row: operation, safe target and state. A permission decision +/// is shown next to the execution state, never in place of it: an allowed +/// call is still running until the tool reports its result. +fn tool_line(appearance: &Appearance, tool: &ToolActivity, expanded: bool) -> Line<'static> { + let label = match &tool.target { + Some(target) => format!("{} {target}", tool.name), + None => tool.name.clone(), + }; + let (status, word) = tool_status(tool.state); + let (status, word) = match (tool.allowed, tool.state) { + (Some(false), _) => (Status::Blocked, "denied".to_string()), + (Some(true), ToolActivityState::Running) => (status, format!("allowed, {word}")), + _ => (status, word.to_string()), + }; + primitives::tool_row(appearance, &label, status, &word, expanded) +} + +/// Rows of input shown per tool in the inspector before truncation. +const DETAIL_OUTPUT_LINES: usize = 6; +const DETAIL_INPUT_CHARS: usize = 300; + +fn tool_detail(appearance: &Appearance, tool: &ToolActivity) -> Vec> { + let indent = |name: &str, value: String| { + Line::from(vec![ + Span::styled(format!(" {name:<8}"), appearance.style(Role::TextMuted)), + Span::styled(value, appearance.style(Role::Text)), + ]) + }; + let mut input: String = tool.input.chars().take(DETAIL_INPUT_CHARS).collect(); + if tool.input.chars().count() > DETAIL_INPUT_CHARS { + input.push_str(" … (truncated)"); + } + let mut lines = vec![indent("input", input.replace('\n', " "))]; + if let Some(output) = &tool.output { + let mut shown = output.lines(); + for (index, line) in shown.by_ref().take(DETAIL_OUTPUT_LINES).enumerate() { + let name = if index == 0 { "output" } else { "" }; + lines.push(indent(name, line.to_string())); + } + let rest = shown.count(); + if rest > 0 { + lines.push(indent("", format!("… {rest} more line(s) not shown"))); + } + } + lines.push(indent( + "owner", + "engine-local tool in this coven-code process".to_string(), + )); + lines +} + +/// Next's approval surface. It replaces the main pane while a request is +/// pending, keeps the draft visible below it, focuses deny by default and +/// names who enforces the decision. Keys are still read by the loop's +/// permission handler; this only draws. +/// The parts of the approval panel, in screen order, before fitting. +struct ApprovalPanel { + title: Line<'static>, + /// Every line of the request's input; never silently shortened. + preview: Vec>, + danger: Option>, + options: Vec>, + details: Vec>, +} + +/// Fit the approval panel into `height` rows. Options are never dropped; +/// then the danger note and title; then as much input as fits, with an +/// explicit count of hidden rows; then supporting details. +fn fit_approval( + appearance: &Appearance, + panel: ApprovalPanel, + width: usize, + height: usize, +) -> Vec> { + let wrap = |lines: Vec>| -> Vec> { + lines + .into_iter() + .flat_map(|line| wrap_line(line, width)) + .collect() + }; + let options = wrap(panel.options); + let danger = wrap(panel.danger.into_iter().collect()); + let title = wrap(vec![panel.title]); + let preview = wrap(panel.preview); + let mut details = wrap(panel.details); + + let mut budget = height; + let mut take = |rows: &[Line<'static>]| -> bool { + if rows.len() <= budget { + budget -= rows.len(); + true + } else { + false + } + }; + let show_options = take(&options); + let show_danger = take(&danger); + let show_title = take(&title); + let shown_preview = if preview.len() <= budget { + preview.len() + } else { + // Reserve one row to say how much of the input is hidden. + budget.saturating_sub(1) + }; + budget -= shown_preview.min(budget); + let hidden = preview.len() - shown_preview; + let marker = if hidden > 0 && budget > 0 { + budget -= 1; + wrap_line( + primitives::status_message( + appearance, + Status::Blocked, + &format!("{hidden} more input rows hidden"), + "enlarge the terminal to read all of it before approving", + ), + width, + ) + .into_iter() + .take(1) + .collect() + } else { + Vec::new() + }; + details.truncate(budget); + + let mut lines = Vec::with_capacity(height); + if show_title { + lines.extend(title); + } + lines.extend(preview.into_iter().take(shown_preview)); + lines.extend(marker); + if show_danger { + lines.extend(danger); + } + if show_options { + lines.extend(options); + } else { + // Too small for every option: keep the focused end, deny. + let skip = options.len().saturating_sub(height); + lines.extend(options.into_iter().skip(skip)); + } + lines.extend(details); + lines.truncate(height); + lines +} + +fn approval_lines( + appearance: &Appearance, + app: &App, + request: &crate::application::view::PendingPermission, +) -> ApprovalPanel { + let title = primitives::inspector_title( + appearance, + &format!("Approve {} (engine-local)", request.tool_name), + ); + let preview = request + .input_preview + .iter() + .flat_map(|preview| preview.lines()) + .map(|line| { + Line::from(Span::styled( + line.to_string(), + appearance.style(Role::Sensitive), + )) + }) + .collect(); + let options: Vec<(char, &str)> = request + .options + .iter() + .map(|(key, label)| (*key, label.as_str())) + .collect(); + let options = primitives::confirmation(appearance, "", &options, request.selected_option, true) + .into_iter() + .skip(1) + .collect(); + let danger = (!request.danger.is_empty()).then(|| { + Line::from(Span::styled( + request.danger.clone(), + appearance.style(Role::Sensitive), + )) + }); + let mut details = Vec::new(); + let armed = app.permission_request.as_ref().is_none_or(|pending| { + !pending.arming || pending.quiet_since.elapsed() >= crate::dialogs::PERMISSION_ARM_DELAY + }); + if !armed { + details.push(primitives::status_message( + appearance, + Status::Pending, + "arming", + "keys typed before this appeared are ignored", + )); + } + if !request.description.is_empty() { + details.push(Line::from(Span::styled( + request.description.clone(), + appearance.style(Role::TextMuted), + ))); + } + details.push(field(appearance, "Project", project_name(app))); + details.push(field( + appearance, + "Enforced by", + "this coven-code process; no daemon protection applies".to_string(), + )); + let consequence = |key: char| match key { + 'y' => Some("this call only"), + 'Y' => Some("this tool or path for the rest of this session"), + 'p' => Some("saves a persistent allow rule to settings for future sessions"), + 'P' => Some("commands with this prefix for the rest of this session"), + 'n' => Some("the call is refused and the model is told"), + _ => None, + }; + for (key, _) in &request.options { + if let Some(text) = consequence(*key) { + details.push(field(appearance, &format!("[{key}]"), text.to_string())); + } + } + details.push(Line::from(Span::styled( + "Allowing authorizes the call; its result is shown when the tool finishes.", + appearance.style(Role::TextSubtle), + ))); + ApprovalPanel { + title, + preview, + danger, + options, + details, } } @@ -700,10 +942,8 @@ fn activity_page(appearance: &Appearance, app: &App, view: &TranscriptView) -> V ))); } for tool in &view.tools { - let (status, word) = tool_status(tool.state); - lines.push(primitives::tool_row( - appearance, &tool.name, status, word, false, - )); + lines.push(tool_line(appearance, tool, true)); + lines.extend(tool_detail(appearance, tool)); } if !app.queued_messages.is_empty() { lines.push(field( diff --git a/src-rust/crates/tui/tests/next_tools.rs b/src-rust/crates/tui/tests/next_tools.rs new file mode 100644 index 00000000..2e42cbd4 --- /dev/null +++ b/src-rust/crates/tui/tests/next_tools.rs @@ -0,0 +1,505 @@ +//! T4 (#411), engine-local slice: tool rows, tool detail, approval safety and +//! hostile text. +//! +//! These fixtures exercise the engine-local permission path only. Protected +//! (daemon-enforced) approvals have no qualified producer contract yet and +//! must render as unavailable, never as a local grant. + +use claurst_api::streaming::ContentDelta; +use claurst_api::AnthropicStreamEvent; +use claurst_core::config::Config; +use claurst_core::cost::CostTracker; +use claurst_core::types::Message; +use claurst_query::QueryEvent; +use claurst_tui::app::App; +use claurst_tui::application::{ScopedQueryEvent, SessionScope, UiRenderer}; +use claurst_tui::dialogs::{handle_permission_key_at, PermissionRequest, PERMISSION_ARM_DELAY}; +use claurst_tui::next::style::{Appearance, ColorMode, Glyphs}; +use claurst_tui::next::{InspectorPage, NextPresentation}; +use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; +use ratatui::backend::TestBackend; +use ratatui::buffer::Buffer; +use ratatui::Terminal; +use std::time::Duration; + +fn next_app() -> App { + let config = Config { + model: Some("claude-tools-model".to_string()), + project_dir: Some("/work/acme-api".into()), + ..Config::default() + }; + let mut app = App::new(config, CostTracker::new()); + app.has_credentials = true; + app.renderer = UiRenderer::Next; + app.next_presentation = NextPresentation { + appearance: Appearance { + color: ColorMode::NoColor, + glyphs: Glyphs::Ascii, + ..Appearance::default() + }, + ..NextPresentation::default() + }; + app.turn_start = None; + app +} + +fn draw(app: &App, width: u16, height: u16) -> Buffer { + let mut terminal = Terminal::new(TestBackend::new(width, height)).expect("terminal"); + terminal + .draw(|frame| claurst_tui::next::render_next(frame, app)) + .expect("draw"); + terminal.backend().buffer().clone() +} + +fn text_of(buffer: &Buffer) -> String { + let width = usize::from(buffer.area.width.max(1)); + buffer + .content + .chunks(width) + .map(|row| { + row.iter() + .map(|cell| cell.symbol()) + .collect::() + .trim_end() + .to_string() + }) + .collect::>() + .join("\n") +} + +fn apply(app: &mut App, event: QueryEvent) { + let scope = SessionScope::default(); + scope.apply( + app, + ScopedQueryEvent { + generation: scope.current(), + event, + }, + ); +} + +fn tool_start(app: &mut App, id: &str, name: &str, input: &str) { + apply( + app, + QueryEvent::ToolStart { + tool_name: name.to_string(), + tool_id: id.to_string(), + input_json: input.to_string(), + }, + ); +} + +fn tool_end(app: &mut App, id: &str, name: &str, result: &str, is_error: bool) { + apply( + app, + QueryEvent::ToolEnd { + tool_name: name.to_string(), + tool_id: id.to_string(), + result: result.to_string(), + is_error, + }, + ); +} + +fn key(code: KeyCode) -> KeyEvent { + KeyEvent::new(code, KeyModifiers::NONE) +} + +fn bash_request() -> PermissionRequest { + PermissionRequest::bash( + "toolu-rm".to_string(), + "Bash".to_string(), + "Delete the build directory".to_string(), + "rm -rf build".to_string(), + None, + ) + .guarded() +} + +fn deny_index(request: &PermissionRequest) -> usize { + request + .options + .iter() + .position(|option| option.key == 'n') + .expect("every request has a deny option") +} + +#[test] +fn a_new_permission_request_defaults_to_deny_and_ignores_keys_until_armed() { + let mut request = bash_request(); + assert_eq!( + request.selected_option, + deny_index(&request), + "default focus is deny" + ); + let shown = request.quiet_since; + + // Keys typed for the composer that land as the prompt appears decide nothing. + for early in [ + key(KeyCode::Enter), + key(KeyCode::Char('y')), + key(KeyCode::Char('1')), + ] { + assert!( + !handle_permission_key_at(&mut request, early, shown + Duration::from_millis(50)), + "{early:?} decided an unarmed request" + ); + assert_eq!(request.selected_option, deny_index(&request)); + } + + // Each ignored key restarts the wait. + let armed = shown + Duration::from_millis(50) + PERMISSION_ARM_DELAY; + // Enter on the default choice denies. + let mut enter = request.clone(); + assert!(handle_permission_key_at( + &mut enter, + key(KeyCode::Enter), + armed + )); + assert_eq!(enter.selected_option, deny_index(&enter)); + // An explicit allow still works once armed. + assert!(handle_permission_key_at( + &mut request, + key(KeyCode::Char('y')), + armed + )); + assert_eq!(request.options[request.selected_option].key, 'y'); +} + +#[test] +fn continuous_typing_or_a_held_key_never_decides_a_request() { + // A user still typing, or key auto-repeat, keeps the request unarmed: + // a decision needs a quiet gap after the last ignored key. + for held in [KeyCode::Char('y'), KeyCode::Enter, KeyCode::Char('p')] { + let mut request = bash_request(); + let shown = request.quiet_since; + for step in 1..=40u64 { + let at = shown + Duration::from_millis(step * 50); + assert!( + !handle_permission_key_at(&mut request, key(held), at), + "{held:?} decided the request {}ms in", + step * 50 + ); + } + assert_eq!(request.selected_option, deny_index(&request)); + let quiet = shown + Duration::from_millis(40 * 50) + PERMISSION_ARM_DELAY; + assert!(handle_permission_key_at( + &mut request, + key(KeyCode::Esc), + quiet + )); + } +} + +#[test] +fn escape_denies_even_before_the_request_is_armed() { + let mut request = bash_request(); + request.selected_option = 0; + let shown = request.quiet_since; + assert!(handle_permission_key_at( + &mut request, + key(KeyCode::Esc), + shown + Duration::from_millis(10) + )); + assert_eq!(request.selected_option, deny_index(&request)); +} + +#[test] +fn hostile_control_sequences_never_reach_the_terminal() { + let mut app = next_app(); + let osc52 = "\u{1b}]52;c;ZXZpbA==\u{7}"; + let clear = "\u{1b}[2J\u{1b}[H"; + let link = "\u{1b}]8;;https://evil.example\u{1b}\\click\u{1b}]8;;\u{1b}\\"; + app.push_message(Message::assistant(format!( + "before {osc52}{clear}{link} after" + ))); + tool_start( + &mut app, + "toolu-x", + "Bash", + &format!(r#"{{"command":"echo {clear}"}}"#), + ); + tool_end( + &mut app, + "toolu-x", + "Bash", + &format!("out {osc52} done"), + false, + ); + for (inspector, width, height) in [ + (None, 80, 24), + (Some(InspectorPage::Activity), 80, 24), + (Some(InspectorPage::Activity), 120, 40), + ] { + app.next_shell.inspector = inspector; + let buffer = draw(&app, width, height); + let bad: Vec<&str> = buffer + .content + .iter() + .map(|cell| cell.symbol()) + .filter(|symbol| symbol.chars().any(char::is_control)) + .collect(); + assert!(bad.is_empty(), "{width}x{height}: control symbols {bad:?}"); + let text = text_of(&buffer); + if inspector.is_none() || width >= 120 { + let joined = text.replace('\n', ""); + assert!( + joined.contains("before") && joined.contains("after"), + "{text}" + ); + } + assert!( + text.contains("^["), + "escapes are shown, not executed:\n{text}" + ); + } +} + +#[test] +fn tool_rows_show_a_safe_target_and_an_honest_state() { + let mut app = next_app(); + app.push_message(Message::user("run the tests".to_string())); + tool_start( + &mut app, + "toolu-1", + "Bash", + r#"{"command":"cargo test -p core\nrm -rf /"}"#, + ); + tool_start( + &mut app, + "toolu-2", + "Read", + r#"{"file_path":"/work/acme-api/src/lib.rs"}"#, + ); + let text = text_of(&draw(&app, 100, 30)); + let bash = text.lines().find(|l| l.contains("Bash")).expect("bash row"); + assert!(bash.contains("cargo test -p core"), "{bash}"); + assert!( + !bash.contains("rm -rf"), + "only the first line is the target: {bash}" + ); + assert!(bash.contains("running"), "{bash}"); + let read = text.lines().find(|l| l.contains("Read")).expect("read row"); + assert!(read.contains("src/lib.rs"), "{read}"); + + tool_end(&mut app, "toolu-2", "Read", "contents", false); + tool_end(&mut app, "toolu-1", "Bash", "1 failed", true); + let text = text_of(&draw(&app, 100, 30)); + assert!( + text.lines() + .any(|l| l.contains("Bash") && l.contains("error")), + "{text}" + ); + assert!( + text.lines() + .any(|l| l.contains("Read") && l.contains("done")), + "{text}" + ); + assert!( + !text + .lines() + .any(|l| l.contains("Read") && l.contains("verified")), + "completion is not verification:\n{text}" + ); +} + +#[test] +fn running_tools_show_cancel_requested_until_the_runtime_settles() { + let mut app = next_app(); + app.push_message(Message::user("go".to_string())); + apply( + &mut app, + QueryEvent::Stream(AnthropicStreamEvent::ContentBlockDelta { + index: 0, + delta: ContentDelta::TextDelta { + text: "working".to_string(), + }, + }), + ); + tool_start(&mut app, "toolu-1", "Bash", r#"{"command":"sleep 100"}"#); + app.handle_key_event(key(KeyCode::Esc)); + assert!(app.take_cancellation_request()); + // Classic clears its tool rows on cancel; Next keeps the evidence. + let text = text_of(&draw(&app, 100, 30)); + assert!( + text.lines() + .any(|l| l.contains("Bash") && l.contains("cancel requested")), + "{text}" + ); + app.settle_cancellation(); + let text = text_of(&draw(&app, 100, 30)); + assert!(!text.contains("cancel requested"), "{text}"); +} + +#[test] +fn the_activity_inspector_shows_sanitized_arguments_output_and_provenance() { + let mut app = next_app(); + tool_start( + &mut app, + "toolu-1", + "Bash", + r#"{"command":"ls -la","description":"List files"}"#, + ); + tool_end(&mut app, "toolu-1", "Bash", "total 0\nREADME.md", false); + app.next_shell.inspector = Some(InspectorPage::Activity); + let text = text_of(&draw(&app, 80, 30)); + assert!(text.contains("ls -la"), "{text}"); + assert!(text.contains("README.md"), "output preview shown:\n{text}"); + assert!( + text.contains("engine-local"), + "execution owner shown:\n{text}" + ); +} + +#[test] +fn next_draws_its_own_approval_with_owner_scope_and_consequences() { + let mut app = next_app(); + app.set_prompt_text("unsent draft".to_string()); + app.permission_request = Some(bash_request()); + assert!( + !claurst_tui::next::delegates_to_classic(&app), + "Next owns the approval surface" + ); + for (width, height) in [(80, 24), (40, 12), (120, 40)] { + let text = text_of(&draw(&app, width, height)); + assert!( + text.contains("rm -rf build"), + "{width}x{height} exact operation:\n{text}" + ); + assert!( + text.contains("engine-local"), + "{width}x{height} enforcement owner:\n{text}" + ); + // The selected (default) option is deny and is marked without color. + assert!( + text.lines() + .any(|l| l.starts_with('>') && l.contains("deny")), + "{width}x{height} deny is focused:\n{text}" + ); + } + let text = text_of(&draw(&app, 120, 40)); + assert!(text.contains("acme-api"), "project shown:\n{text}"); + assert!( + text.contains("persistent"), + "consequence of always-allow shown:\n{text}" + ); + assert!( + text.contains("unsent draft"), + "draft stays visible:\n{text}" + ); +} + +#[test] +fn an_authorization_is_not_shown_as_a_successful_execution() { + let mut app = next_app(); + tool_start( + &mut app, + "toolu-rm", + "Bash", + r#"{"command":"rm -rf build"}"#, + ); + app.record_permission_decision("toolu-rm", true); + let text = text_of(&draw(&app, 100, 30)); + let row = text.lines().find(|l| l.contains("Bash")).expect("row"); + assert!(row.contains("allowed") && row.contains("running"), "{row}"); + tool_end( + &mut app, + "toolu-rm", + "Bash", + "permission ok but command failed", + true, + ); + let text = text_of(&draw(&app, 100, 30)); + let row = text.lines().find(|l| l.contains("Bash")).expect("row"); + assert!(row.contains("error"), "the execution result wins: {row}"); + + let mut denied = next_app(); + tool_start(&mut denied, "toolu-2", "Write", r#"{"file_path":"a.txt"}"#); + denied.record_permission_decision("toolu-2", false); + let text = text_of(&draw(&denied, 100, 30)); + assert!( + text.lines() + .any(|l| l.contains("Write") && l.contains("denied")), + "{text}" + ); +} + +#[test] +fn a_cancelled_call_closing_result_does_not_add_a_second_failed_row() { + use claurst_core::types::{ContentBlock, ToolResultContent}; + let mut app = next_app(); + app.push_message(Message::user("go".to_string())); + let mut asked = Message::assistant_blocks(vec![ContentBlock::ToolUse { + id: "toolu-1".to_string(), + name: "Bash".to_string(), + input: serde_json::json!({"command": "sleep 100"}), + }]); + asked.uuid = Some("asked-1".to_string()); + apply(&mut app, QueryEvent::DurableMessage { message: asked }); + tool_start(&mut app, "toolu-1", "Bash", r#"{"command":"sleep 100"}"#); + app.handle_key_event(key(KeyCode::Esc)); + assert!(app.take_cancellation_request()); + // The engine closes every cancelled tool use with one durable error. + let mut closing = Message::user_blocks(vec![ContentBlock::ToolResult { + tool_use_id: "toolu-1".to_string(), + content: ToolResultContent::Text("cancelled".to_string()), + is_error: Some(true), + }]); + closing.uuid = Some("closing-1".to_string()); + apply(&mut app, QueryEvent::DurableMessage { message: closing }); + app.settle_cancellation(); + let text = text_of(&draw(&app, 100, 30)); + let rows: Vec<&str> = text.lines().filter(|l| l.contains("sleep 100")).collect(); + assert_eq!(rows.len(), 1, "{text}"); + assert!(rows[0].contains("cancelled"), "{text}"); +} + +fn multiline_request(lines: usize) -> PermissionRequest { + let mut command: Vec = (1..lines).map(|n| format!("echo step {n}")).collect(); + command.push("rm -rf ~/precious".to_string()); + PermissionRequest::bash( + "toolu-multi".to_string(), + "Bash".to_string(), + "Run a script".to_string(), + command.join("\n"), + None, + ) + .guarded() +} + +#[test] +fn the_approval_shows_the_whole_command_or_says_what_is_hidden() { + // Room for everything: every line, including the last, is visible. + let mut app = next_app(); + app.permission_request = Some(multiline_request(6)); + let text = text_of(&draw(&app, 120, 40)); + assert!(text.contains("echo step 3"), "{text}"); + assert!(text.contains("rm -rf ~/precious"), "{text}"); + + // Too long to fit: the hidden part is announced and no option is lost. + app.permission_request = Some(multiline_request(60)); + for (width, height) in [(80, 24), (120, 40), (40, 12)] { + let text = text_of(&draw(&app, width, height)); + if !text.contains("rm -rf ~/precious") { + assert!( + text.contains("more input rows hidden"), + "{width}x{height}:\n{text}" + ); + } + let request = app.permission_request.as_ref().expect("request"); + let deny = &request.options[deny_index(request)].label; + assert!( + text.contains(deny.as_str()), + "{width}x{height} lost deny:\n{text}" + ); + if height >= 24 { + for option in &request.options { + assert!( + text.contains(option.label.as_str()), + "{width}x{height} lost {:?}:\n{text}", + option.label + ); + } + } + } +} diff --git a/src-rust/crates/tui/tests/presentation_parity.rs b/src-rust/crates/tui/tests/presentation_parity.rs index 637cb8bd..f9c6f4f2 100644 --- a/src-rust/crates/tui/tests/presentation_parity.rs +++ b/src-rust/crates/tui/tests/presentation_parity.rs @@ -125,7 +125,7 @@ fn both_renderers_show_the_projected_transcript_and_live_output() { } let next = draw(&app, UiRenderer::Next, 120, 40); assert!(next.contains(claurst_tui::next::PREVIEW_LABEL), "{next}"); - assert!(next.contains("▸ Read ○ done"), "{next}"); + assert!(next.contains("▸ Read ENGINE-CONTRACT.md ○ done"), "{next}"); } fn bash_request(id: &str) -> PermissionRequest { @@ -200,12 +200,13 @@ fn renderers_produce_identical_outgoing_operations_for_the_same_input() { } #[test] -fn next_draws_permission_requests_through_classic() { +fn next_draws_permission_requests_itself_unless_another_surface_is_open() { let (mut app, _) = replay(trace()); app.permission_request = Some(bash_request("toolu-parity-3")); - assert!(claurst_tui::next::delegates_to_classic(&app)); + // #411: Next owns the approval surface. + assert!(!claurst_tui::next::delegates_to_classic(&app)); let next = draw(&app, UiRenderer::Next, 120, 40); - assert_eq!(next, draw(&app, UiRenderer::Classic, 120, 40)); + assert!(next.contains("cargo test --workspace"), "{next}"); let request = app.permission_request.as_ref().expect("request"); for option in &request.options { assert!( @@ -214,6 +215,13 @@ fn next_draws_permission_requests_through_classic() { option.label ); } + // With another input-owning surface open, Classic's precedence applies. + app.help_overlay.visible = true; + assert!(claurst_tui::next::delegates_to_classic(&app)); + assert_eq!( + draw(&app, UiRenderer::Next, 120, 40), + draw(&app, UiRenderer::Classic, 120, 40) + ); } /// Opens one input-owning surface on an App.