diff --git a/crates/tui/src/core/engine/turn_loop.rs b/crates/tui/src/core/engine/turn_loop.rs index c5b099ed2..f8f1b7de4 100644 --- a/crates/tui/src/core/engine/turn_loop.rs +++ b/crates/tui/src/core/engine/turn_loop.rs @@ -1660,6 +1660,7 @@ impl Engine { .send(Event::ApprovalRequired { id: tool_id.clone(), tool_name: tool_name.clone(), + input: tool_input.clone(), description: plan.approval_description.clone(), approval_key, approval_grouping_key, diff --git a/crates/tui/src/core/events.rs b/crates/tui/src/core/events.rs index b02ba2f99..22f0172e7 100644 --- a/crates/tui/src/core/events.rs +++ b/crates/tui/src/core/events.rs @@ -225,6 +225,7 @@ pub enum Event { ApprovalRequired { id: String, tool_name: String, + input: serde_json::Value, description: String, /// Exact-argument fingerprint, used to scope *denials* (#1617). approval_key: String, diff --git a/crates/tui/src/runtime_threads.rs b/crates/tui/src/runtime_threads.rs index 787142ba4..6afe78881 100644 --- a/crates/tui/src/runtime_threads.rs +++ b/crates/tui/src/runtime_threads.rs @@ -4160,6 +4160,7 @@ mod tests { approval_grouping_key: "test_key".to_string(), id: "tool_stale".to_string(), tool_name: "exec_command".to_string(), + input: json!({"command": "stale approval"}), description: "stale approval".to_string(), }) .await?; @@ -4233,6 +4234,7 @@ mod tests { approval_grouping_key: "key1".to_string(), id: "tool_external_allow".to_string(), tool_name: "exec_command".to_string(), + input: json!({"command": "external allow"}), description: "external allow".to_string(), }) .await?; @@ -4310,6 +4312,7 @@ mod tests { approval_grouping_key: "key2".to_string(), id: "tool_external_deny".to_string(), tool_name: "exec_command".to_string(), + input: json!({"command": "external deny"}), description: "external deny".to_string(), }) .await?; @@ -4496,6 +4499,7 @@ mod tests { approval_grouping_key: "key3".to_string(), id: "tool_remember".to_string(), tool_name: "exec_command".to_string(), + input: json!({"command": "remember=true"}), description: "remember=true".to_string(), }) .await?; diff --git a/crates/tui/src/tui/approval.rs b/crates/tui/src/tui/approval.rs index dc7e9cdcd..f2f98b2a6 100644 --- a/crates/tui/src/tui/approval.rs +++ b/crates/tui/src/tui/approval.rs @@ -28,14 +28,20 @@ //! module always assumes the user is being asked. use crate::localization::Locale; +use crate::palette; use crate::sandbox::SandboxPolicy; use crate::tui::views::{ModalKind, ModalView, ViewAction, ViewEvent}; use crate::tui::widgets::{ApprovalWidget, ElevationWidget, Renderable}; -use crossterm::event::{KeyCode, KeyEvent}; +use crossterm::event::{KeyCode, KeyEvent, KeyModifiers, MouseEvent, MouseEventKind}; +use ratatui::style::{Modifier, Style}; +use ratatui::text::{Line, Span}; use serde_json::Value; +use std::cell::{Cell, RefCell}; use std::path::{Path, PathBuf}; use std::time::{Duration, Instant}; +const DIFF_PREVIEW_MAX_BYTES: u64 = 512 * 1024; + /// Determines when tool executions require user approval #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] pub enum ApprovalMode { @@ -113,6 +119,71 @@ pub enum RiskLevel { Destructive, } +/// Cached diff preview for file-modification tools. +/// +/// Built once at `ApprovalRequest` construction time so the modal doesn't +/// re-read the file every render frame. The variants make the "nothing to +/// show" cases explicit instead of collapsing to `None` and silently hiding +/// the preview panel (#1638). +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ApprovalDiffPreview { + /// Normal unified diff against an existing file (or apply_patch content). + Diff { + text: String, + added: usize, + deleted: usize, + }, + /// `write_file` against a path that doesn't exist yet — there's no old + /// content to diff against, so we show the proposed content as additions. + NewFile { path: String, content: String }, + /// Content matches the file already — render a calm "no changes" hint + /// instead of swallowing the whole preview area. + NoChange { path: String }, + /// `edit_file` search string not present in the file — render a warning + /// plus a search→replace fallback diff so the user still sees intent. + MissingMatch { + path: String, + text: String, + match_count: usize, + }, + /// Existing file is large enough that building a full in-memory diff would + /// risk a visible pause in the TUI event loop. + SkippedLargeFile { path: String, size: u64, limit: u64 }, +} + +impl ApprovalDiffPreview { + /// Plain unified-diff text suitable for the pager / detail view. + /// Returns an empty string for non-diff variants. + #[must_use] + pub fn diff_text(&self) -> &str { + match self { + Self::Diff { text, .. } | Self::MissingMatch { text, .. } => text, + Self::NewFile { content, .. } => content, + Self::NoChange { .. } | Self::SkippedLargeFile { .. } => "", + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ApprovalDetail { + pub label: String, + pub value: String, + pub shell_lines: Option>, +} + +#[derive(Debug, Clone)] +pub struct RenderedDiffPanel { + pub header: String, + pub lines: Vec>, +} + +#[derive(Debug, Clone)] +struct DiffPanelCache { + width: u16, + locale: Locale, + panel: RenderedDiffPanel, +} + /// Request for user approval of a tool execution #[derive(Debug, Clone)] pub struct ApprovalRequest { @@ -120,14 +191,10 @@ pub struct ApprovalRequest { pub id: String, /// Tool being executed pub tool_name: String, - /// Human-readable tool description from the engine - pub description: String, /// Tool category pub category: ToolCategory, /// Stakes-based routing for the takeover modal pub risk: RiskLevel, - /// Derived impact summary for the approval prompt - pub impacts: Vec, /// Tool parameters (for display) pub params: Value, /// Exact-argument fingerprint, used to scope *denials* (#1617). @@ -135,54 +202,89 @@ pub struct ApprovalRequest { /// Lossy / arity-aware fingerprint, used to scope *approvals* so an /// "approve for session" covers later flag variants (v0.8.37). pub approval_grouping_key: String, + /// Snapshot of the diff/preview state, built once at construction so + /// renderers never re-read the filesystem mid-render. + diff_preview: Option, + /// Precomputed popup details, including parsed shell command lines. + prominent_details: Vec, } impl ApprovalRequest { + #[cfg(test)] pub fn new( id: &str, tool_name: &str, description: &str, params: &Value, approval_key: &str, + ) -> Self { + Self::new_with_workspace(id, tool_name, description, params, approval_key, None) + } + + pub fn new_with_workspace( + id: &str, + tool_name: &str, + _description: &str, + params: &Value, + approval_key: &str, + workspace: Option, ) -> Self { let category = get_tool_category(tool_name); let risk = classify_risk(tool_name, category, params); let approval_grouping_key = crate::tools::approval_cache::build_approval_grouping_key(tool_name, params).0; + // Build snapshots once. Renderers read these caches instead of doing + // path canonicalization, shell parsing, or filesystem reads per frame. + let prominent_details = build_prominent_details(category, params, workspace.as_deref()); + let diff_preview = build_diff_preview(tool_name, params, workspace.as_deref()); Self { id: id.to_string(), tool_name: tool_name.to_string(), - description: description.to_string(), category, risk, - impacts: build_impact_summary(tool_name, category, params), params: params.clone(), approval_key: approval_key.to_string(), approval_grouping_key, + diff_preview, + prominent_details, } } - /// Format parameters for display (truncated) - pub fn params_display(&self) -> String { - let truncated = truncate_params_value(&self.params, 200); - serde_json::to_string(&truncated).unwrap_or_else(|_| truncated.to_string()) + /// Extract the most important param values for the approval widget. + #[must_use] + pub fn prominent_detail_items(&self, locale: Locale) -> Vec { + self.prominent_details + .iter() + .map(|detail| ApprovalDetail { + label: localize_detail_label(&detail.label, locale).to_string(), + value: detail.value.clone(), + shell_lines: detail.shell_lines.clone(), + }) + .collect() } - pub fn description_for_locale(&self, locale: Locale) -> String { - match locale { - Locale::ZhHans => localized_description_zh_hans(self.category), - _ => self.description.clone(), - } + /// Cached diff/preview snapshot, or `None` when the tool isn't a file + /// modification. Building happens once at request construction; never + /// re-reads the filesystem. + #[must_use] + pub fn diff_preview(&self) -> Option<&ApprovalDiffPreview> { + self.diff_preview.as_ref() } +} - pub fn impacts_for_locale(&self, locale: Locale) -> Vec { - match locale { - Locale::ZhHans => { - build_impact_summary_zh_hans(&self.tool_name, self.category, &self.params) - } - _ => self.impacts.clone(), - } +fn localize_detail_label(label: &str, locale: Locale) -> &str { + match locale { + Locale::ZhHans => match label { + "Command" => "命令", + "Dir" => "目录", + "File" => "文件", + "Path" => "路径", + "Target" => "目标", + "Input" => "输入", + _ => label, + }, + _ => label, } } @@ -192,7 +294,10 @@ pub fn get_tool_category(name: &str) -> ToolCategory { ToolCategory::FileWrite } else if matches!(name, "web_run" | "web_search" | "fetch_url") { ToolCategory::Network - } else if name == "exec_shell" { + } else if matches!( + name, + "exec_shell" | "task_shell_start" | "exec_shell_wait" | "exec_shell_interact" + ) { ToolCategory::Shell } else if name.starts_with("list_mcp_") || name.starts_with("read_mcp_") @@ -246,7 +351,11 @@ pub fn classify_risk(tool_name: &str, category: ToolCategory, params: &Value) -> // shape so a future routing tweak (say, pure-readonly `ls` // staying benign) lands here without a second pass. ToolCategory::Shell => { - if let Some(cmd) = params.get("command").and_then(Value::as_str) { + if let Some(cmd) = params + .get("command") + .or_else(|| params.get("cmd")) + .and_then(Value::as_str) + { let _ = crate::command_safety::analyze_command(cmd); } RiskLevel::Destructive @@ -259,193 +368,522 @@ pub fn classify_risk(tool_name: &str, category: ToolCategory, params: &Value) -> } } -fn param_preview(params: &Value, keys: &[&str], max_len: usize) -> Option { +/// Like [`param_preview`] but never truncates the string value. Used for +/// shell commands so the popup shows what's actually being run instead of +/// `...`-eliding the dangerous tail. The popup body uses `Paragraph::wrap` +/// so long values fold across multiple visual lines on their own. +fn param_text(params: &Value, keys: &[&str]) -> Option { let Value::Object(map) = params else { return None; }; - for key in keys { - let Some(value) = map.get(*key) else { - continue; - }; - match value { - Value::String(text) => return Some(truncate_string_value(text, max_len)), - Value::Number(number) => return Some(number.to_string()), - Value::Bool(flag) => return Some(flag.to_string()), - Value::Array(items) if !items.is_empty() => { - let preview = items - .iter() - .take(3) - .map(|item| match item { - Value::String(text) => truncate_string_value(text, max_len / 2), - other => truncate_string_value(&other.to_string(), max_len / 2), - }) - .collect::>() - .join(", "); - return Some(truncate_string_value(&preview, max_len)); - } - other => return Some(truncate_string_value(&other.to_string(), max_len)), + if let Some(Value::String(text)) = map.get(*key) { + return Some(text.clone()); } } - None } -fn mcp_server_hint(tool_name: &str) -> Option { - let remainder = tool_name.strip_prefix("mcp_")?; - let (server, _) = remainder.split_once('_')?; - if server.is_empty() { - None - } else { - Some(server.to_string()) - } -} - -fn build_impact_summary(tool_name: &str, category: ToolCategory, params: &Value) -> Vec { +fn build_prominent_details( + category: ToolCategory, + params: &Value, + workspace: Option<&str>, +) -> Vec { + let mut details = Vec::new(); match category { - ToolCategory::Safe => { - let mut impacts = vec!["Read-only operation.".to_string()]; - if let Some(path) = param_preview(params, &["path", "ref_id", "uri"], 72) { - impacts.push(format!("Reads: {path}")); + ToolCategory::Shell => { + if let Some(cmd) = param_text(params, &["command", "cmd"]) { + let shell_lines = format_shell_command_for_approval(&cmd); + details.push(ApprovalDetail { + label: "Command".into(), + value: cmd, + shell_lines: Some(shell_lines), + }); + } + if let Some(raw_dir) = param_text(params, &["workdir", "cwd"]) + .or_else(|| param_preview(params, &["workdir", "cwd"], 96)) + { + let is_current = workspace.is_some_and(|ws| paths_equivalent(&raw_dir, ws)); + let value = if is_current { + "(current)".to_string() + } else { + truncate_string_value(&raw_dir, 96) + }; + details.push(ApprovalDetail { + label: "Dir".into(), + value, + shell_lines: None, + }); } - impacts } ToolCategory::FileWrite => { - let mut impacts = - vec!["Writes files in the workspace or an approved write scope.".to_string()]; - if let Some(path) = param_preview(params, &["path", "target", "destination"], 72) { - impacts.push(format!("Writes: {path}")); + if let Some(path) = param_preview(params, &["path", "target", "destination"], 200) { + details.push(ApprovalDetail { + label: "File".into(), + value: path, + shell_lines: None, + }); } - impacts } - ToolCategory::Shell => { - let mut impacts = vec!["Executes a shell command.".to_string()]; - if let Some(command) = param_preview(params, &["cmd", "command"], 96) { - impacts.push(format!("Command: {command}")); - } - if let Some(workdir) = param_preview(params, &["workdir", "cwd"], 72) { - impacts.push(format!("Working dir: {workdir}")); + ToolCategory::Safe => { + if let Some(path) = param_preview(params, &["path", "ref_id", "uri"], 200) { + details.push(ApprovalDetail { + label: "Path".into(), + value: path, + shell_lines: None, + }); } - impacts } ToolCategory::Network => { - let mut impacts = vec!["May reach network services or remote content.".to_string()]; if let Some(target) = - param_preview(params, &["url", "q", "query", "location", "repo"], 96) + param_preview(params, &["url", "q", "query", "location", "repo"], 200) { - impacts.push(format!("Target: {target}")); + details.push(ApprovalDetail { + label: "Target".into(), + value: target, + shell_lines: None, + }); } - impacts } - ToolCategory::McpRead => { - let mut impacts = - vec!["Reads from an MCP server without an obvious local write.".to_string()]; - if let Some(server) = mcp_server_hint(tool_name) { - impacts.push(format!("Server: {server}")); + _ => { + if let Some(val) = param_preview( + params, + &["command", "path", "url", "q", "query", "ref_id"], + 200, + ) { + details.push(ApprovalDetail { + label: "Input".into(), + value: val, + shell_lines: None, + }); } - impacts } - ToolCategory::McpAction => { - let mut impacts = - vec!["Calls an MCP server action that may have side effects.".to_string()]; - if let Some(server) = mcp_server_hint(tool_name) { - impacts.push(format!("Server: {server}")); + } + details +} + +fn paths_equivalent(left: &str, right: &str) -> bool { + let left = Path::new(left); + let right = Path::new(right); + left.canonicalize().unwrap_or_else(|_| left.to_path_buf()) + == right.canonicalize().unwrap_or_else(|_| right.to_path_buf()) +} + +pub(crate) fn format_shell_command_for_approval(command: &str) -> Vec { + if let Some(write) = parse_printf_write_file_command(command) { + return format_printf_write_file_preview(write); + } + + let mut out = Vec::new(); + for raw in command.split('\n') { + let mut current = String::new(); + let mut chars = raw.chars().peekable(); + let mut quote: Option = None; + while let Some(ch) = chars.next() { + if matches!(ch, '"' | '\'') { + if quote == Some(ch) { + quote = None; + } else if quote.is_none() { + quote = Some(ch); + } + current.push(ch); + continue; } - impacts - } - ToolCategory::Unknown => { - let mut impacts = vec![ - "Tool is not classified. Review params carefully before approving.".to_string(), - ]; - if let Some(target) = param_preview( - params, - &["path", "cmd", "command", "url", "q", "query", "ref_id"], - 96, - ) { - impacts.push(format!("Primary input: {target}")); + + if quote.is_none() && ch == '&' && chars.peek() == Some(&'&') { + chars.next(); + push_shell_clause(&mut out, &mut current, Some("&&")); + continue; } - impacts + + if quote.is_none() && ch == '|' { + push_shell_clause(&mut out, &mut current, Some("|")); + continue; + } + + if quote.is_none() && ch == ';' { + current.push(';'); + push_shell_clause(&mut out, &mut current, None); + continue; + } + + if quote.is_none() && matches!(ch, '>' | '<') { + current.push(ch); + while chars + .peek() + .is_some_and(|next| matches!(next, '>' | '<' | '&')) + { + current.push(chars.next().expect("peeked char exists")); + } + while chars.peek().is_some_and(|next| next.is_whitespace()) { + current.push(chars.next().expect("peeked char exists")); + } + while chars + .peek() + .is_some_and(|next| !next.is_whitespace() && !matches!(next, '&' | '|' | ';')) + { + current.push(chars.next().expect("peeked char exists")); + } + continue; + } + + current.push(ch); } + push_shell_clause(&mut out, &mut current, None); + } + + if out.is_empty() { + vec![String::new()] + } else { + out } } -fn localized_description_zh_hans(category: ToolCategory) -> String { - match category { - ToolCategory::Safe => "请求执行只读操作。".to_string(), - ToolCategory::FileWrite => "请求修改文件。请确认路径和内容符合预期。".to_string(), - ToolCategory::Shell => "请求执行 shell 命令。请先检查命令和工作目录。".to_string(), - ToolCategory::Network => "请求访问网络或远程内容。请确认目标可信。".to_string(), - ToolCategory::McpRead => "请求从 MCP 服务器读取信息。".to_string(), - ToolCategory::McpAction => "请求调用 MCP 服务器操作，可能产生副作用。".to_string(), - ToolCategory::Unknown => "请求运行未分类工具。批准前请仔细检查参数。".to_string(), +struct PrintfWriteFilePreview { + target: String, + lines: Vec, +} + +fn parse_printf_write_file_command(command: &str) -> Option { + let (before_redirect, after_redirect) = split_unquoted_redirect(command)?; + let before_redirect = before_redirect.trim(); + if !before_redirect.starts_with("printf") { + return None; + } + let tokens = shlex::split(before_redirect)?; + if tokens.len() < 3 || tokens.first()?.as_str() != "printf" { + return None; } + let fmt = tokens.get(1)?.as_str(); + if fmt != "%s\n" && fmt != "%s\\n" { + return None; + } + let target_tokens = shlex::split(after_redirect.trim_start_matches(['>', '&']).trim_start())?; + if target_tokens.len() != 1 { + return None; + } + let target = target_tokens.into_iter().next()?; + let lines = tokens.into_iter().skip(2).collect::>(); + if lines.is_empty() { + return None; + } + Some(PrintfWriteFilePreview { target, lines }) } -fn build_impact_summary_zh_hans( - tool_name: &str, - category: ToolCategory, - params: &Value, -) -> Vec { - match category { - ToolCategory::Safe => { - let mut impacts = vec!["只读操作。".to_string()]; - if let Some(path) = param_preview(params, &["path", "ref_id", "uri"], 72) { - impacts.push(format!("读取：{path}")); +fn format_printf_write_file_preview(write: PrintfWriteFilePreview) -> Vec { + let mut out = Vec::new(); + out.push("Write file via printf".to_string()); + out.push(format!("Target: {}", write.target)); + out.push(format!("Lines: {}", write.lines.len())); + out.push(String::new()); + out.push("Content:".to_string()); + let total = write.lines.len(); + for (idx, line) in write.lines.into_iter().enumerate().take(12) { + out.push(format!("{:>4} {}", idx + 1, line)); + } + if total > 12 { + out.push(format!(" ... (+{} more lines)", total - 12)); + } + out +} + +fn split_unquoted_redirect(command: &str) -> Option<(&str, &str)> { + let mut quote: Option = None; + for (idx, ch) in command.char_indices() { + if matches!(ch, '"' | '\'') { + if quote == Some(ch) { + quote = None; + } else if quote.is_none() { + quote = Some(ch); } - impacts + continue; } - ToolCategory::FileWrite => { - let mut impacts = vec!["会写入工作区或已批准写入范围内的文件。".to_string()]; - if let Some(path) = param_preview(params, &["path", "target", "destination"], 72) { - impacts.push(format!("写入：{path}")); - } - impacts + if quote.is_none() && ch == '>' { + let before = &command[..idx]; + let after = &command[idx + ch.len_utf8()..]; + return Some((before, after)); } - ToolCategory::Shell => { - let mut impacts = vec!["执行 shell 命令。".to_string()]; - if let Some(command) = param_preview(params, &["cmd", "command"], 96) { - impacts.push(format!("命令：{command}")); + } + None +} + +fn push_shell_clause(out: &mut Vec, current: &mut String, connector: Option<&str>) { + let trimmed = current.trim(); + if !trimmed.is_empty() { + let mut line = trimmed.to_string(); + if let Some(connector) = connector { + line.push(' '); + line.push_str(connector); + } + out.push(line); + } else if let Some(connector) = connector + && let Some(prev) = out.last_mut() + && !prev.ends_with(connector) + { + prev.push(' '); + prev.push_str(connector); + } + current.clear(); +} + +/// Resolve a tool-supplied path against the workspace when it's relative. +/// Absolute paths are returned unchanged so `write_file` to `/etc/foo` still +/// shows the right diff. The original string flows through if there's no +/// workspace context — matching the previous behavior for tests / direct +/// constructors. +fn resolve_workspace_path(raw: &str, workspace: Option<&str>) -> std::path::PathBuf { + let path = Path::new(raw); + if path.is_absolute() { + return path.to_path_buf(); + } + match workspace { + Some(ws) => Path::new(ws).join(path), + None => path.to_path_buf(), + } +} + +/// Count `+` and `-` lines in a unified diff. Delegates to the shared +/// `summarize_diff` so the popup header reads the same `+N -M` totals +/// the detail pager shows in its summary section — keeps the two views +/// agreeing on what "changed" means even for tricky inputs (no-newline +/// markers, multi-file patches, etc.). +fn count_diff_changes(diff: &str) -> (usize, usize) { + let summaries = crate::tui::diff_render::summarize_diff(diff); + if summaries.is_empty() { + // summarize_diff only collects files that have a `diff --git` or + // `+++` header. For single-file fragments produced by + // `make_unified_diff` we fall back to a plain line scan so the + // header still reflects the change. + let mut added = 0usize; + let mut deleted = 0usize; + for line in diff.lines() { + if line.starts_with("+++") || line.starts_with("---") { + continue; } - if let Some(workdir) = param_preview(params, &["workdir", "cwd"], 72) { - impacts.push(format!("工作目录：{workdir}")); + if line.starts_with('+') { + added += 1; + } else if line.starts_with('-') { + deleted += 1; } - impacts } - ToolCategory::Network => { - let mut impacts = vec!["可能访问网络服务或远程内容。".to_string()]; - if let Some(target) = - param_preview(params, &["url", "q", "query", "location", "repo"], 96) - { - impacts.push(format!("目标：{target}")); + return (added, deleted); + } + let added = summaries.iter().map(|s| s.added).sum(); + let deleted = summaries.iter().map(|s| s.deleted).sum(); + (added, deleted) +} + +enum PreviewFileRead { + Content(String), + Skipped(ApprovalDiffPreview), + Unreadable, +} + +fn read_file_for_diff(resolved: &Path, display_path: &str) -> PreviewFileRead { + if let Ok(metadata) = std::fs::metadata(resolved) + && metadata.is_file() + && metadata.len() > DIFF_PREVIEW_MAX_BYTES + { + return PreviewFileRead::Skipped(ApprovalDiffPreview::SkippedLargeFile { + path: display_path.to_string(), + size: metadata.len(), + limit: DIFF_PREVIEW_MAX_BYTES, + }); + } + + match std::fs::read_to_string(resolved) { + Ok(content) => PreviewFileRead::Content(content), + Err(_) => PreviewFileRead::Unreadable, + } +} + +/// Build the diff snapshot for an approval request. Reads the filesystem +/// at most once per request — relative paths resolve against `workspace` +/// so previews work when the agent is rooted elsewhere from the TUI's CWD. +pub fn build_diff_preview( + tool_name: &str, + params: &Value, + workspace: Option<&str>, +) -> Option { + match tool_name { + "edit_file" => { + let path = params.get("path")?.as_str()?; + let search = params.get("search")?.as_str()?; + let replace = params.get("replace")?.as_str()?; + let resolved = resolve_workspace_path(path, workspace); + match read_file_for_diff(&resolved, path) { + PreviewFileRead::Content(file) => { + let count = file.matches(search).count(); + if count == 0 { + // search isn't present — render the search/replace pair + // as a fallback diff so the user still sees the intent, + // and flag it so the UI can warn. + let text = + crate::tools::diff_format::make_unified_diff(path, search, replace); + Some(ApprovalDiffPreview::MissingMatch { + path: path.to_string(), + text, + match_count: 0, + }) + } else { + // Simulate the replace and diff the full file so the + // user sees the actual change in context. + let updated = file.replacen(search, replace, 1); + let text = + crate::tools::diff_format::make_unified_diff(path, &file, &updated); + if text.is_empty() { + Some(ApprovalDiffPreview::NoChange { + path: path.to_string(), + }) + } else { + let (added, deleted) = count_diff_changes(&text); + Some(ApprovalDiffPreview::Diff { + text, + added, + deleted, + }) + } + } + } + PreviewFileRead::Skipped(preview) => Some(preview), + PreviewFileRead::Unreadable => { + // File missing — fall back to inputs-only diff. edit_file + // would fail at execution anyway, so MissingMatch is the + // honest framing. + let text = crate::tools::diff_format::make_unified_diff(path, search, replace); + Some(ApprovalDiffPreview::MissingMatch { + path: path.to_string(), + text, + match_count: 0, + }) + } } - impacts } - ToolCategory::McpRead => { - let mut impacts = vec!["从 MCP 服务器读取信息，不应产生本地写入。".to_string()]; - if let Some(server) = mcp_server_hint(tool_name) { - impacts.push(format!("服务器：{server}")); + "write_file" => { + let path = params.get("path")?.as_str()?; + let new_content = params.get("content")?.as_str()?; + let resolved = resolve_workspace_path(path, workspace); + match read_file_for_diff(&resolved, path) { + PreviewFileRead::Content(old_content) => { + let text = crate::tools::diff_format::make_unified_diff( + path, + &old_content, + new_content, + ); + if text.is_empty() { + Some(ApprovalDiffPreview::NoChange { + path: path.to_string(), + }) + } else { + let (added, deleted) = count_diff_changes(&text); + Some(ApprovalDiffPreview::Diff { + text, + added, + deleted, + }) + } + } + PreviewFileRead::Skipped(preview) => Some(preview), + PreviewFileRead::Unreadable => Some(ApprovalDiffPreview::NewFile { + path: path.to_string(), + content: new_content.to_string(), + }), } - impacts } - ToolCategory::McpAction => { - let mut impacts = vec!["调用可能产生副作用的 MCP 服务器操作。".to_string()]; - if let Some(server) = mcp_server_hint(tool_name) { - impacts.push(format!("服务器：{server}")); + "apply_patch" => { + if let Some(patch) = params.get("patch").and_then(|v| v.as_str()) { + if patch.is_empty() { + None + } else { + let (added, deleted) = count_diff_changes(patch); + Some(ApprovalDiffPreview::Diff { + text: patch.to_string(), + added, + deleted, + }) + } + } else if let Some(changes) = params.get("changes").and_then(|v| v.as_array()) { + // `changes` is an array of `{path, content}` full-file + // replacements. Build a multi-file unified diff against the + // current contents so the approval shows the same shape as + // the `patch` path. + let mut out = String::new(); + for change in changes { + let Some(path) = change.get("path").and_then(|v| v.as_str()) else { + continue; + }; + let Some(new_content) = change.get("content").and_then(|v| v.as_str()) else { + continue; + }; + let resolved = resolve_workspace_path(path, workspace); + let old_content = match read_file_for_diff(&resolved, path) { + PreviewFileRead::Content(content) => content, + PreviewFileRead::Skipped(preview) => return Some(preview), + PreviewFileRead::Unreadable => String::new(), + }; + let fragment = crate::tools::diff_format::make_unified_diff( + path, + &old_content, + new_content, + ); + if !fragment.is_empty() { + if !out.is_empty() { + out.push('\n'); + } + // Synthesize a `diff --git` header so the multi-file + // summary in `diff_render` picks the file up. + out.push_str(&format!("diff --git a/{path} b/{path}\n")); + out.push_str(&fragment); + } + } + if out.is_empty() { + None + } else { + let (added, deleted) = count_diff_changes(&out); + Some(ApprovalDiffPreview::Diff { + text: out, + added, + deleted, + }) + } + } else { + None } - impacts } - ToolCategory::Unknown => { - let mut impacts = vec!["工具未分类。批准前请仔细检查参数。".to_string()]; - if let Some(target) = param_preview( - params, - &["path", "cmd", "command", "url", "q", "query", "ref_id"], - 96, - ) { - impacts.push(format!("主要输入：{target}")); + _ => None, + } +} + +fn param_preview(params: &Value, keys: &[&str], max_len: usize) -> Option { + let Value::Object(map) = params else { + return None; + }; + + for key in keys { + let Some(value) = map.get(*key) else { + continue; + }; + match value { + Value::String(text) => return Some(truncate_string_value(text, max_len)), + Value::Number(number) => return Some(number.to_string()), + Value::Bool(flag) => return Some(flag.to_string()), + Value::Array(items) if !items.is_empty() => { + let preview = items + .iter() + .take(3) + .map(|item| match item { + Value::String(text) => truncate_string_value(text, max_len / 2), + other => truncate_string_value(&other.to_string(), max_len / 2), + }) + .collect::>() + .join(", "); + return Some(truncate_string_value(&preview, max_len)); } - impacts + other => return Some(truncate_string_value(&other.to_string(), max_len)), } } + + None } /// Indices into the option list shared by both variants. Visible to @@ -512,6 +950,10 @@ pub struct ApprovalView { requested_at: Instant, /// Whether the approval card is collapsed to a single-line banner. pub(crate) collapsed: bool, + diff_scroll: Cell, + diff_total_lines: Cell, + diff_visible_lines: Cell, + diff_panel_cache: RefCell>, } impl ApprovalView { @@ -529,6 +971,10 @@ impl ApprovalView { timeout: None, requested_at: Instant::now(), collapsed: false, + diff_scroll: Cell::new(0), + diff_total_lines: Cell::new(0), + diff_visible_lines: Cell::new(0), + diff_panel_cache: RefCell::new(None), } } @@ -576,6 +1022,64 @@ impl ApprovalView { self.locale } + pub(crate) fn set_diff_metrics(&self, total: usize, visible: usize) -> usize { + self.diff_total_lines.set(total); + self.diff_visible_lines.set(visible); + let max_scroll = total.saturating_sub(visible); + let scroll = self.diff_scroll.get().min(max_scroll); + self.diff_scroll.set(scroll); + scroll + } + + pub(crate) fn cached_diff_panel( + &self, + width: u16, + locale: Locale, + ) -> Option { + let cache = self.diff_panel_cache.borrow(); + let cache = cache.as_ref()?; + if cache.width == width && cache.locale == locale { + Some(cache.panel.clone()) + } else { + None + } + } + + pub(crate) fn set_cached_diff_panel( + &self, + width: u16, + locale: Locale, + panel: RenderedDiffPanel, + ) { + *self.diff_panel_cache.borrow_mut() = Some(DiffPanelCache { + width, + locale, + panel, + }); + } + + fn scroll_diff_up(&mut self, amount: usize) { + self.diff_scroll + .set(self.diff_scroll.get().saturating_sub(amount)); + self.pending_confirm = None; + } + + fn scroll_diff_down(&mut self, amount: usize) { + let visible = self.diff_visible_lines.get(); + let max_scroll = self.diff_total_lines.get().saturating_sub(visible); + self.diff_scroll + .set((self.diff_scroll.get() + amount).min(max_scroll)); + self.pending_confirm = None; + } + + fn diff_page_height(&self) -> usize { + self.diff_visible_lines.get().max(1) + } + + fn diff_half_page_height(&self) -> usize { + self.diff_page_height().div_ceil(2).max(1) + } + /// Try to commit (or stage) the given option respecting the /// variant's confirmation policy. Returns the action the modal /// stack should apply. @@ -608,12 +1112,103 @@ impl ApprovalView { } fn emit_params_pager(&self) -> ViewAction { - let content = serde_json::to_string_pretty(&self.request.params) - .unwrap_or_else(|_| self.request.params.to_string()); - ViewAction::Emit(ViewEvent::OpenTextPager { - title: format!("Tool Params: {}", self.request.tool_name), - content, - }) + if let Some(lines) = self.build_detail_lines() { + ViewAction::Emit(ViewEvent::OpenStyledPager { + title: format!("Details: {}", self.request.tool_name), + lines, + }) + } else { + let content = serde_json::to_string_pretty(&self.request.params) + .unwrap_or_else(|_| self.request.params.to_string()); + ViewAction::Emit(ViewEvent::OpenTextPager { + title: format!("Tool Params: {}", self.request.tool_name), + content, + }) + } + } + + /// Build a styled details view for file tools. + fn build_detail_lines(&self) -> Option>> { + let label_style = Style::default() + .fg(palette::DEEPSEEK_SKY) + .add_modifier(Modifier::BOLD); + let muted_style = Style::default().fg(palette::TEXT_MUTED); + let mut lines: Vec> = Vec::new(); + + match self.request.tool_name.as_str() { + "edit_file" => { + let path = self.request.params.get("path")?.as_str()?; + lines.push(Line::from(vec![ + Span::styled("File: ", muted_style), + Span::raw(path.to_string()), + ])); + push_approval_changes(&mut lines, self.request.diff_preview(), 120); + if let Some(search) = self.request.params.get("search").and_then(|v| v.as_str()) { + push_multiline_field(&mut lines, "Search", search, label_style); + } + if let Some(replace) = self.request.params.get("replace").and_then(|v| v.as_str()) { + push_multiline_field(&mut lines, "Replace", replace, label_style); + } + if let Some(fuzz) = self.request.params.get("fuzz") { + push_scalar_field(&mut lines, "Fuzz", &fuzz.to_string(), label_style); + } + Some(lines) + } + "write_file" => { + let path = self.request.params.get("path")?.as_str()?; + lines.push(Line::from(vec![ + Span::styled("File: ", muted_style), + Span::raw(path.to_string()), + ])); + push_approval_changes(&mut lines, self.request.diff_preview(), 120); + if let Some(content) = self.request.params.get("content").and_then(|v| v.as_str()) { + push_multiline_field(&mut lines, "Content", content, label_style); + } + if let Some(mode) = self.request.params.get("mode").and_then(|v| v.as_str()) { + push_scalar_field(&mut lines, "Mode", mode, label_style); + } + Some(lines) + } + "apply_patch" => { + if let Some(path) = self.request.params.get("path").and_then(|v| v.as_str()) { + lines.push(Line::from(vec![ + Span::styled("File: ", muted_style), + Span::raw(path.to_string()), + ])); + } + push_approval_changes(&mut lines, self.request.diff_preview(), 120); + if let Some(patch) = self.request.params.get("patch").and_then(|v| v.as_str()) { + push_multiline_field(&mut lines, "Patch", patch, label_style); + } + if let Some(changes) = self + .request + .params + .get("changes") + .and_then(|v| v.as_array()) + { + lines.push(Line::from(Span::styled("Changes:", label_style))); + for (idx, change) in changes.iter().enumerate() { + let Some(change_path) = change.get("path").and_then(|v| v.as_str()) else { + continue; + }; + lines.push(Line::from(vec![ + Span::raw(" "), + Span::styled( + format!("{}.", idx + 1), + Style::default().fg(palette::TEXT_MUTED), + ), + Span::raw(" "), + Span::styled(change_path.to_string(), muted_style), + ])); + if let Some(content) = change.get("content").and_then(|v| v.as_str()) { + push_multiline_body(&mut lines, content, 4); + } + } + } + Some(lines) + } + _ => None, + } } fn is_timed_out(&self) -> bool { @@ -634,16 +1229,47 @@ impl ModalView for ApprovalView { } fn handle_key(&mut self, key: KeyEvent) -> ViewAction { + let ctrl = key.modifiers.contains(KeyModifiers::CONTROL); + if ctrl { + match key.code { + KeyCode::Char('u') | KeyCode::Char('U') => { + self.scroll_diff_up(self.diff_half_page_height()); + return ViewAction::None; + } + KeyCode::Char('d') | KeyCode::Char('D') => { + self.scroll_diff_down(self.diff_half_page_height()); + return ViewAction::None; + } + _ => {} + } + } + match key.code { KeyCode::Tab => { self.collapsed = !self.collapsed; ViewAction::None } - KeyCode::Up | KeyCode::Char('k') => { + KeyCode::PageUp => { + self.scroll_diff_up(self.diff_page_height()); + ViewAction::None + } + KeyCode::PageDown => { + self.scroll_diff_down(self.diff_page_height()); + ViewAction::None + } + KeyCode::Up => { + self.scroll_diff_up(1); + ViewAction::None + } + KeyCode::Down => { + self.scroll_diff_down(1); + ViewAction::None + } + KeyCode::Char('k') => { self.select_prev(); ViewAction::None } - KeyCode::Down | KeyCode::Char('j') => { + KeyCode::Char('j') => { self.select_next(); ViewAction::None } @@ -675,6 +1301,20 @@ impl ModalView for ApprovalView { } } + fn handle_mouse(&mut self, mouse: MouseEvent) -> ViewAction { + match mouse.kind { + MouseEventKind::ScrollUp => { + self.scroll_diff_up(3); + ViewAction::None + } + MouseEventKind::ScrollDown => { + self.scroll_diff_down(3); + ViewAction::None + } + _ => ViewAction::None, + } + } + fn render(&self, area: ratatui::layout::Rect, buf: &mut ratatui::buffer::Buffer) { let approval_widget = ApprovalWidget::new(&self.request, self); approval_widget.render(area, buf); @@ -688,31 +1328,75 @@ impl ModalView for ApprovalView { } } -fn truncate_params_value(value: &Value, max_len: usize) -> Value { - match value { - Value::Object(map) => { - let truncated = map - .iter() - .map(|(key, val)| (key.clone(), truncate_params_value(val, max_len))) - .collect(); - Value::Object(truncated) - } - Value::Array(items) => { - let truncated_items = items - .iter() - .map(|val| truncate_params_value(val, max_len)) - .collect(); - Value::Array(truncated_items) - } - Value::String(text) => Value::String(truncate_string_value(text, max_len)), - other => { - let rendered = other.to_string(); - if rendered.chars().count() > max_len { - Value::String(truncate_string_value(&rendered, max_len)) +fn push_approval_changes( + lines: &mut Vec>, + preview: Option<&ApprovalDiffPreview>, + width: u16, +) { + let label_style = Style::default() + .fg(palette::DEEPSEEK_SKY) + .add_modifier(Modifier::BOLD); + let muted_style = Style::default().fg(palette::TEXT_MUTED); + + lines.push(Line::from("")); + lines.push(Line::from(Span::styled("Changes:", label_style))); + match preview { + Some(ApprovalDiffPreview::Diff { text, .. }) + | Some(ApprovalDiffPreview::MissingMatch { text, .. }) => { + if text.is_empty() { + lines.push(Line::from(Span::styled( + " (no textual changes - content matches current file)".to_string(), + muted_style, + ))); } else { - other.clone() + lines.extend(crate::tui::diff_render::render_diff(text, width)); } } + Some(ApprovalDiffPreview::NewFile { path, content }) => { + let diff = crate::tools::diff_format::make_unified_diff(path, "", content); + lines.extend(crate::tui::diff_render::render_diff(&diff, width)); + } + Some(ApprovalDiffPreview::SkippedLargeFile { size, limit, .. }) => { + lines.push(Line::from(Span::styled( + format!(" (diff preview skipped - file is {size} bytes, limit is {limit} bytes)"), + muted_style, + ))); + } + Some(ApprovalDiffPreview::NoChange { .. }) | None => { + lines.push(Line::from(Span::styled( + " (no textual changes - content matches current file)".to_string(), + muted_style, + ))); + } + } + lines.push(Line::from("")); +} + +fn push_scalar_field(lines: &mut Vec>, label: &str, value: &str, label_style: Style) { + lines.push(Line::from(vec![ + Span::raw(" "), + Span::styled(format!("{label}: "), label_style), + Span::raw(value.to_string()), + ])); +} + +fn push_multiline_field( + lines: &mut Vec>, + label: &str, + value: &str, + label_style: Style, +) { + lines.push(Line::from(vec![ + Span::raw(" "), + Span::styled(format!("{label}:"), label_style), + ])); + push_multiline_body(lines, value, 4); +} + +fn push_multiline_body(lines: &mut Vec>, value: &str, indent: usize) { + let indent = " ".repeat(indent); + for raw in value.split('\n') { + lines.push(Line::from(Span::raw(format!("{indent}{raw}")))); } } @@ -998,6 +1682,12 @@ mod tests { #[test] fn test_get_tool_category_shell_tools() { assert_eq!(get_tool_category("exec_shell"), ToolCategory::Shell); + assert_eq!(get_tool_category("task_shell_start"), ToolCategory::Shell); + assert_eq!(get_tool_category("exec_shell_wait"), ToolCategory::Shell); + assert_eq!( + get_tool_category("exec_shell_interact"), + ToolCategory::Shell + ); assert_eq!( get_tool_category("mcp_linear_save_issue"), ToolCategory::McpAction @@ -1097,61 +1787,348 @@ mod tests { } #[test] - fn test_approval_request_params_display_truncates() { - let long_content = "x".repeat(300); - let params = json!({"path": "src/main.rs", "content": long_content}); + fn test_prominent_details_shell() { + let params = json!({"command": "npm run build", "cwd": "/home/user"}); let request = ApprovalRequest::new( "test-id", - "write_file", - "Write a file to disk", + "exec_shell", + "Run a shell command", ¶ms, "test_key", ); - - let display = request.params_display(); - assert!(display.len() < 250); - assert!(display.contains("src/main.rs")); + let details = request.prominent_detail_items(Locale::En); + assert_eq!(details[0].label, "Command"); + assert_eq!(details[0].value, "npm run build"); + assert_eq!( + details[0].shell_lines.as_deref(), + Some(&["npm run build".to_string()][..]) + ); + assert_eq!(details[1].label, "Dir"); + assert_eq!(details[1].value, "/home/user"); } #[test] - fn test_approval_request_params_display_short() { - let params = json!({"path": "src/main.rs"}); - let request = ApprovalRequest::new( - "test-id", - "read_file", - "Read a file from disk", - ¶ms, - "test_key", + fn test_prominent_details_shell_does_not_truncate_long_command() { + // Regression: shell commands used to be hard-clipped at 120 chars + // with a trailing `…`, hiding the dangerous tail of long pipelines + // (the part where `rm -rf` or `>` redirects usually live). The + // popup body wraps long lines via `Paragraph::wrap`, so we now + // pass the command through verbatim. + let cmd = format!("printf '{}\n' > /tmp/x && cat /tmp/x", "x".repeat(300)); + let params = json!({"command": cmd, "cwd": "/home/user"}); + let request = + ApprovalRequest::new("test-id", "exec_shell", "Run shell", ¶ms, "test_key"); + let details = request.prominent_detail_items(Locale::En); + assert_eq!(details[0].label, "Command"); + assert_eq!( + details[0].value, cmd, + "shell command must be returned verbatim, no `…` truncation", ); - - let display = request.params_display(); - assert!(display.contains("src/main.rs")); + let shell_lines = details[0] + .shell_lines + .as_ref() + .expect("shell command lines should be cached"); + assert!(shell_lines.iter().any(|line| line.contains("cat /tmp/x"))); } #[test] - fn test_approval_request_derives_impact_summary() { - let params = json!({"cmd": "cargo test", "workdir": "/tmp/project"}); - let request = ApprovalRequest::new( + fn test_prominent_details_shell_marks_current_dir() { + let params = json!({"command": "ls", "cwd": "/home/user/project"}); + let request = ApprovalRequest::new_with_workspace( "test-id", "exec_shell", "Run a shell command", ¶ms, "test_key", + Some("/home/user/project".to_string()), ); + let details = request.prominent_detail_items(Locale::En); + assert_eq!(details[1].label, "Dir"); + assert_eq!(details[1].value, "(current)"); + } - assert_eq!(request.category, ToolCategory::Shell); + #[test] + fn test_prominent_details_file_write() { + let params = json!({"path": "src/main.rs", "content": "fn main() {}"}); + let request = + ApprovalRequest::new("test-id", "write_file", "Write file", ¶ms, "test_key"); + let details = request.prominent_detail_items(Locale::En); + assert_eq!(details[0].label, "File"); + assert_eq!(details[0].value, "src/main.rs"); + } + + #[test] + fn test_diff_preview_edit_file() { + let params = json!({ + "path": "src/main.rs", + "search": "fn main() {\n println!(\"hello\");\n}", + "replace": "fn main() {\n println!(\"world\");\n}" + }); + let request = + ApprovalRequest::new("test-id", "edit_file", "Edit file", ¶ms, "test_key"); + let preview = request + .diff_preview() + .expect("edit_file produces a preview"); + // Tests don't see src/main.rs, so we land in the MissingMatch fallback + // which still surfaces a search→replace diff for visual confirmation. + let diff = preview.diff_text(); + assert!(diff.contains("--- a/src/main.rs")); + assert!(diff.contains("+++ b/src/main.rs")); + assert!(diff.contains("- println!(\"hello\");")); + assert!(diff.contains("+ println!(\"world\");")); assert!( - request - .impacts - .iter() - .any(|line| line.contains("Executes a shell command")) + matches!(preview, ApprovalDiffPreview::MissingMatch { .. }), + "expected MissingMatch when file is absent, got {preview:?}" ); + } + + #[test] + fn test_diff_preview_edit_file_existing_simulates_replace() { + // When the file exists and search matches, we should produce a full + // simulated diff against the real file content. + let path = std::env::temp_dir().join("deepseek_test_edit_file_existing.txt"); + std::fs::write(&path, "alpha\nbeta\ngamma\n").unwrap(); + let params = json!({ + "path": path.display().to_string(), + "search": "beta", + "replace": "BETA", + }); + let request = + ApprovalRequest::new("test-id", "edit_file", "Edit file", ¶ms, "test_key"); + let preview = request.diff_preview().expect("edit_file preview"); + match preview { + ApprovalDiffPreview::Diff { text, .. } => { + assert!(text.contains("-beta"), "got {text}"); + assert!(text.contains("+BETA"), "got {text}"); + } + other => panic!("expected Diff for existing edit_file, got {other:?}"), + } + let _ = std::fs::remove_file(&path); + } + + #[test] + fn test_build_detail_lines_edit_file_structures_multiline_params() { + let params = json!({ + "path": "test_db.js", + "search": "async transaction(callback) {\n const conn = await this.pool.getConnection();\n}", + "replace": "async transaction(callback, timeout = 30000) {\n const conn = await this.pool.getConnection();\n}", + }); + let request = + ApprovalRequest::new("test-id", "edit_file", "Edit file", ¶ms, "test_key"); + let view = ApprovalView::new(request); + let lines = view.build_detail_lines().expect("file tools have details"); + let rendered = lines + .iter() + .map(|line| { + line.spans + .iter() + .map(|span| span.content.as_ref()) + .collect::() + }) + .collect::>() + .join("\n"); + assert!(rendered.contains("File: test_db.js"), "{rendered}"); + assert!(rendered.contains("Changes:"), "{rendered}"); + assert!(rendered.contains("Search:"), "{rendered}"); + assert!(rendered.contains("Replace:"), "{rendered}"); + assert!(!rendered.contains("\"search\""), "{rendered}"); + assert!(!rendered.contains("\"replace\""), "{rendered}"); + } + + #[test] + fn test_diff_preview_write_file_existing() { + let path = std::env::temp_dir().join("deepseek_test_diff_preview.txt"); + std::fs::write(&path, "old content\n").unwrap(); + let params = json!({"path": path.display().to_string(), "content": "new content\n"}); + let request = + ApprovalRequest::new("test-id", "write_file", "Write file", ¶ms, "test_key"); + let preview = request + .diff_preview() + .expect("write_file on existing file should produce a preview"); + let diff = preview.diff_text(); + assert!(diff.contains("-old content"), "{diff}"); + assert!(diff.contains("+new content"), "{diff}"); assert!( - request - .impacts - .iter() - .any(|line| line.contains("cargo test")) + matches!(preview, ApprovalDiffPreview::Diff { .. }), + "expected Diff variant, got {preview:?}" ); + let _ = std::fs::remove_file(&path); + } + + #[test] + fn test_diff_preview_write_file_large_existing_skips_inline_diff() { + let path = std::env::temp_dir().join("deepseek_test_diff_large_file.txt"); + std::fs::write(&path, "x".repeat((DIFF_PREVIEW_MAX_BYTES + 1) as usize)).unwrap(); + let params = json!({"path": path.display().to_string(), "content": "new content +"}); + let request = + ApprovalRequest::new("test-id", "write_file", "Write file", ¶ms, "test_key"); + let preview = request + .diff_preview() + .expect("large existing file should still produce a preview state"); + match preview { + ApprovalDiffPreview::SkippedLargeFile { size, limit, .. } => { + assert!(*size > *limit); + assert_eq!(*limit, DIFF_PREVIEW_MAX_BYTES); + } + other => panic!("expected SkippedLargeFile, got {other:?}"), + } + let _ = std::fs::remove_file(&path); + } + + #[test] + fn test_diff_preview_write_file_unchanged_shows_no_change() { + // write_file with content identical to the file's current bytes used + // to drop the whole preview panel. We now surface a NoChange hint so + // the user knows the call is a no-op. + let path = std::env::temp_dir().join("deepseek_test_diff_no_change.txt"); + std::fs::write(&path, "same\n").unwrap(); + let params = json!({"path": path.display().to_string(), "content": "same\n"}); + let request = + ApprovalRequest::new("test-id", "write_file", "Write file", ¶ms, "test_key"); + let preview = request.diff_preview().expect("NoChange is still a preview"); + assert!( + matches!(preview, ApprovalDiffPreview::NoChange { .. }), + "expected NoChange, got {preview:?}" + ); + let _ = std::fs::remove_file(&path); + } + + #[test] + fn test_diff_preview_write_file_new() { + let path = std::env::temp_dir().join("deepseek_test_diff_new_file.txt"); + let _ = std::fs::remove_file(&path); + let params = json!({"path": path.display().to_string(), "content": "brand new\n"}); + let request = + ApprovalRequest::new("test-id", "write_file", "Write file", ¶ms, "test_key"); + let preview = request + .diff_preview() + .expect("write_file on new file should produce a preview"); + match preview { + ApprovalDiffPreview::NewFile { content, .. } => { + assert!(content.contains("brand new"), "{content}"); + } + other => panic!("expected NewFile variant, got {other:?}"), + } + } + + #[test] + fn test_diff_preview_write_file_resolves_workspace_relative_path() { + // Regression for the bug where write_file with a workspace-relative + // path produced an empty preview because std::fs::read_to_string was + // called with the raw relative path. + let workspace = std::env::temp_dir().join("deepseek_test_ws_relative"); + std::fs::create_dir_all(&workspace).unwrap(); + let file_rel = "nested/file.txt"; + let abs = workspace.join(file_rel); + std::fs::create_dir_all(abs.parent().unwrap()).unwrap(); + std::fs::write(&abs, "before\n").unwrap(); + + let params = json!({"path": file_rel, "content": "after\n"}); + let request = ApprovalRequest::new_with_workspace( + "test-id", + "write_file", + "Write file", + ¶ms, + "test_key", + Some(workspace.display().to_string()), + ); + let preview = request.diff_preview().expect("preview built"); + match preview { + ApprovalDiffPreview::Diff { text, .. } => { + assert!(text.contains("-before"), "{text}"); + assert!(text.contains("+after"), "{text}"); + } + other => panic!("expected Diff for resolved path, got {other:?}"), + } + let _ = std::fs::remove_dir_all(&workspace); + } + + #[test] + fn test_diff_preview_apply_patch() { + let patch = "--- a/f.rs\n+++ b/f.rs\n@@ -1 +1 @@\n-old\n+new\n"; + let params = json!({"path": "f.rs", "patch": patch}); + let request = + ApprovalRequest::new("test-id", "apply_patch", "Apply patch", ¶ms, "test_key"); + let preview = request.diff_preview().expect("apply_patch preview"); + match preview { + ApprovalDiffPreview::Diff { text, .. } => assert_eq!(text, patch), + other => panic!("expected Diff variant for apply_patch, got {other:?}"), + } + } + + #[test] + fn test_diff_preview_apply_patch_changes_array() { + // apply_patch accepts a `changes` array as a full-file replacement + // alternative to `patch`. The preview must surface those changes + // instead of leaving the popup blank. + let workspace = std::env::temp_dir().join("deepseek_test_apply_patch_changes"); + std::fs::create_dir_all(&workspace).unwrap(); + let a = workspace.join("a.txt"); + std::fs::write(&a, "old\n").unwrap(); + + let params = json!({ + "changes": [ + {"path": "a.txt", "content": "new\n"}, + {"path": "b.txt", "content": "added\n"}, + ] + }); + let request = ApprovalRequest::new_with_workspace( + "test-id", + "apply_patch", + "Apply patch", + ¶ms, + "test_key", + Some(workspace.display().to_string()), + ); + let preview = request + .diff_preview() + .expect("changes array should produce a preview"); + match preview { + ApprovalDiffPreview::Diff { text, .. } => { + assert!(text.contains("diff --git a/a.txt b/a.txt"), "{text}"); + assert!(text.contains("-old"), "{text}"); + assert!(text.contains("+new"), "{text}"); + assert!(text.contains("diff --git a/b.txt b/b.txt"), "{text}"); + assert!(text.contains("+added"), "{text}"); + } + other => panic!("expected Diff for changes, got {other:?}"), + } + let _ = std::fs::remove_dir_all(&workspace); + } + + #[test] + fn test_build_detail_lines_apply_patch_changes_without_top_level_path() { + let params = json!({ + "changes": [ + {"path": "a.txt", "content": "new\n"}, + ] + }); + let request = + ApprovalRequest::new("test-id", "apply_patch", "Apply patch", ¶ms, "test_key"); + let view = ApprovalView::new(request); + let lines = view.build_detail_lines().expect("apply_patch has details"); + let rendered = lines + .iter() + .map(|line| { + line.spans + .iter() + .map(|span| span.content.as_ref()) + .collect::() + }) + .collect::>() + .join("\n"); + assert!(rendered.contains("Changes:"), "{rendered}"); + assert!(rendered.contains("a.txt"), "{rendered}"); + assert!(!rendered.contains("\"changes\""), "{rendered}"); + } + + #[test] + fn test_diff_preview_none_for_other_tools() { + let params = json!({"command": "ls"}); + let request = + ApprovalRequest::new("test-id", "exec_shell", "Run shell", ¶ms, "test_key"); + assert!(request.diff_preview().is_none()); } // ======================================================================== @@ -1335,13 +2312,13 @@ mod tests { assert_eq!(view.selected, 0); // clamped at 0 view.handle_key(create_key_event(KeyCode::Down)); - assert_eq!(view.selected, 1); + assert_eq!(view.selected, 0); view.handle_key(create_key_event(KeyCode::Char('j'))); - assert_eq!(view.selected, 2); + assert_eq!(view.selected, 1); view.handle_key(create_key_event(KeyCode::Char('k'))); - assert_eq!(view.selected, 1); + assert_eq!(view.selected, 0); } #[test] @@ -1350,17 +2327,41 @@ mod tests { let action = view.handle_key(create_key_event(KeyCode::Char('v'))); assert!(matches!( action, - ViewAction::Emit(ViewEvent::OpenTextPager { .. }) + ViewAction::Emit(ViewEvent::OpenTextPager { .. } | ViewEvent::OpenStyledPager { .. }) )); let mut view = ApprovalView::new(benign_request()); let action = view.handle_key(create_key_event(KeyCode::Char('V'))); assert!(matches!( action, - ViewAction::Emit(ViewEvent::OpenTextPager { .. }) + ViewAction::Emit(ViewEvent::OpenTextPager { .. } | ViewEvent::OpenStyledPager { .. }) )); } + #[test] + fn test_approval_view_view_params_uses_styled_pager_for_files() { + let mut view = ApprovalView::new(destructive_request()); + let action = view.handle_key(create_key_event(KeyCode::Char('v'))); + match action { + ViewAction::Emit(ViewEvent::OpenStyledPager { title, lines }) => { + assert!(title.contains("Details")); + let rendered = lines + .iter() + .map(|line| { + line.spans + .iter() + .map(|span| span.content.as_ref()) + .collect::() + }) + .collect::>() + .join("\n"); + assert!(rendered.contains("Changes:")); + assert!(rendered.contains("@@")); + } + other => panic!("expected styled pager, got {other:?}"), + } + } + #[test] fn test_approval_view_current_decision_mapping() { let mut view = ApprovalView::new(benign_request()); @@ -1566,7 +2567,9 @@ mod tests { joined.contains("Single key approves"), "benign hint missing:\n{joined}" ); - assert!(joined.contains("read_file")); + assert!(joined.contains("Safe")); + assert!(joined.contains("Path")); + assert!(joined.contains("src/main.rs")); } #[test] @@ -1582,7 +2585,13 @@ mod tests { joined.contains("Two keys to approve"), "destructive hint missing:\n{joined}" ); - assert!(joined.contains("write_file")); + assert!(joined.contains("File Write")); + assert!(joined.contains("File")); + assert!(joined.contains("src/main.rs")); + assert!( + joined.contains("Approve file writes this session"), + "session approval label missing:\n{joined}" + ); } #[test] @@ -1595,12 +2604,32 @@ mod tests { joined.contains("Confirm destructive action"), "confirm banner missing:\n{joined}" ); + assert!( + joined.contains("Confirm file:"), + "confirm detail missing:\n{joined}" + ); assert!( joined.contains("(staged)"), "stage marker missing:\n{joined}" ); } + #[test] + fn up_down_scroll_diff_without_changing_selection() { + let mut view = ApprovalView::new(destructive_request()); + let before = view.selected(); + assert!(matches!( + view.handle_key(create_key_event(KeyCode::Up)), + ViewAction::None + )); + assert_eq!(view.selected(), before); + assert!(matches!( + view.handle_key(create_key_event(KeyCode::Down)), + ViewAction::None + )); + assert_eq!(view.selected(), before); + } + #[test] fn render_destructive_zh_hans_localizes_security_copy() { let mut view = ApprovalView::new_for_locale(destructive_request(), Locale::ZhHans); @@ -1618,18 +2647,19 @@ mod tests { joined.contains("文件写入"), "missing zh category:\n{joined}" ); + assert!(joined.contains("文件"), "missing zh file label:\n{joined}"); assert!( - joined.contains("影响："), - "missing zh impact label:\n{joined}" - ); - assert!( - joined.contains("写入：src/main.rs"), - "missing zh impact path:\n{joined}" + joined.contains("src/main.rs"), + "missing file path:\n{joined}" ); assert!( joined.contains("仅本次批准"), "missing zh approve option:\n{joined}" ); + assert!( + joined.contains("本会话自动批准文件写入"), + "missing zh session approve option:\n{joined}" + ); view.handle_key(create_key_event(KeyCode::Char('y'))); let lines = render_lines(&view, 100, 40); @@ -1638,6 +2668,10 @@ mod tests { joined.contains("确认破坏性操作"), "missing zh confirm banner:\n{joined}" ); + assert!( + joined.contains("确认文件："), + "missing zh confirm detail:\n{joined}" + ); assert!( joined.contains("(待确认)"), "missing zh staged marker:\n{joined}" diff --git a/crates/tui/src/tui/diff_render.rs b/crates/tui/src/tui/diff_render.rs index ac8cb7bca..c0c982bda 100644 --- a/crates/tui/src/tui/diff_render.rs +++ b/crates/tui/src/tui/diff_render.rs @@ -39,15 +39,15 @@ pub fn render_diff(diff: &str, width: u16) -> Vec> { if raw.starts_with("@@") { if let Some((old_start, new_start)) = parse_hunk_header(raw) { - old_line = Some(old_start); - new_line = Some(new_start); + old_line = old_start; + new_line = new_start; } lines.extend(render_hunk_header(raw, width)); continue; } if raw.starts_with('+') && !raw.starts_with("+++") { - let content = raw.trim_start_matches('+'); + let content = raw.strip_prefix('+').unwrap_or(raw); lines.extend(render_diff_line( content, width, @@ -65,7 +65,7 @@ pub fn render_diff(diff: &str, width: u16) -> Vec> { } if raw.starts_with('-') && !raw.starts_with("---") { - let content = raw.trim_start_matches('-'); + let content = raw.strip_prefix('-').unwrap_or(raw); lines.extend(render_diff_line( content, width, @@ -83,7 +83,7 @@ pub fn render_diff(diff: &str, width: u16) -> Vec> { } if raw.starts_with(' ') { - let content = raw.trim_start_matches(' '); + let content = raw.strip_prefix(' ').unwrap_or(raw); lines.extend(render_diff_line( content, width, @@ -107,6 +107,95 @@ pub fn render_diff(diff: &str, width: u16) -> Vec> { lines } +/// Render a unified diff in the **compact** layout used by the approval +/// popup: keeps `@@` hunk headers, line numbers, and `+/-` colouring, but +/// drops the multi-line summary block and the `--- a/... +++ b/...` / +/// `diff --git` headers. That trim is what frees up the ~6 rows the old +/// renderer was burning on metadata inside a 10-line preview window. +/// +/// The full renderer (`render_diff`) is still what the detail pager uses, +/// so nothing here changes what the user sees behind `v`. +pub fn render_diff_compact(diff: &str, width: u16) -> Vec> { + let mut lines = Vec::new(); + let mut old_line: Option = None; + let mut new_line: Option = None; + + for raw in diff.lines() { + if raw.starts_with("diff --git") + || raw.starts_with("index ") + || raw.starts_with("--- ") + || raw.starts_with("+++ ") + { + continue; + } + + if raw.starts_with("@@") { + if let Some((old_start, new_start)) = parse_hunk_header(raw) { + old_line = old_start; + new_line = new_start; + } + lines.extend(render_hunk_header(raw, width)); + continue; + } + + if raw.starts_with('+') { + let content = raw.strip_prefix('+').unwrap_or(raw); + lines.extend(render_diff_line( + content, + width, + old_line, + new_line, + '+', + Style::default() + .fg(palette::DIFF_ADDED) + .bg(palette::DIFF_ADDED_BG), + )); + if let Some(line) = new_line.as_mut() { + *line = line.saturating_add(1); + } + continue; + } + + if raw.starts_with('-') { + let content = raw.strip_prefix('-').unwrap_or(raw); + lines.extend(render_diff_line( + content, + width, + old_line, + new_line, + '-', + Style::default() + .fg(palette::STATUS_ERROR) + .bg(palette::DIFF_DELETED_BG), + )); + if let Some(line) = old_line.as_mut() { + *line = line.saturating_add(1); + } + continue; + } + + if raw.starts_with(' ') { + let content = raw.strip_prefix(' ').unwrap_or(raw); + lines.extend(render_diff_line( + content, + width, + old_line, + new_line, + ' ', + Style::default().fg(palette::TEXT_PRIMARY), + )); + if let Some(line) = old_line.as_mut() { + *line = line.saturating_add(1); + } + if let Some(line) = new_line.as_mut() { + *line = line.saturating_add(1); + } + } + } + + lines +} + #[must_use] pub fn summarize_diff(diff: &str) -> Vec { let mut summaries = Vec::new(); @@ -254,15 +343,26 @@ fn render_diff_summary(summaries: &[DiffFileSummary], width: u16) -> Vec Option<(usize, usize)> { +fn parse_hunk_header(line: &str) -> Option<(Option, Option)> { let parts: Vec<&str> = line.split_whitespace().collect(); if parts.len() < 3 { return None; } let old = parts[1].trim_start_matches('-'); let new = parts[2].trim_start_matches('+'); - let old_start = old.split(',').next()?.parse::().ok()?; - let new_start = new.split(',').next()?.parse::().ok()?; + // 0 means "no lines" (e.g. new file) → None so the gutter stays blank. + let old_start = old + .split(',') + .next()? + .parse::() + .ok() + .filter(|&v| v > 0); + let new_start = new + .split(',') + .next()? + .parse::() + .ok() + .filter(|&v| v > 0); Some((old_start, new_start)) } @@ -345,33 +445,14 @@ fn wrap_text(text: &str, width: usize) -> Vec { let mut current = String::new(); let mut current_width = 0; - for word in text.split_whitespace() { - let word_width = word.width(); - if word_width > width { - if !current.is_empty() { - lines.push(std::mem::take(&mut current)); - current_width = 0; - } - push_word_breaking_chars(word, width, &mut current, &mut current_width, &mut lines); - continue; - } - let additional = if current.is_empty() { - word_width - } else { - word_width + 1 - }; - if current_width + additional > width && !current.is_empty() { - lines.push(current); - current = word.to_string(); - current_width = word_width; - } else { - if !current.is_empty() { - current.push(' '); - current_width += 1; - } - current.push_str(word); - current_width += word_width; + for ch in text.chars() { + let char_width = ch.width().unwrap_or(1); + if current_width + char_width > width && current_width > 0 { + lines.push(std::mem::take(&mut current)); + current_width = 0; } + current.push(ch); + current_width += char_width; } if current.is_empty() { @@ -383,24 +464,6 @@ fn wrap_text(text: &str, width: usize) -> Vec { lines } -fn push_word_breaking_chars( - word: &str, - width: usize, - current: &mut String, - current_width: &mut usize, - lines: &mut Vec, -) { - for ch in word.chars() { - let char_width = ch.width().unwrap_or(1); - if *current_width + char_width > width && *current_width > 0 { - lines.push(std::mem::take(current)); - *current_width = 0; - } - current.push(ch); - *current_width += char_width; - } -} - #[cfg(test)] mod tests { use super::*; @@ -456,7 +519,8 @@ diff --git a/src/a.rs b/src/a.rs let rendered = render_diff(diff, 80); let text = rendered.iter().map(line_text).collect::>(); assert!(text[0].contains("summary: 1 file, +1 -1, 1 hunk")); - assert!(text.iter().any(|line| line.contains("src/a.rs +1 -1"))); + assert!(text.iter().any(|line| line.contains("src/a.rs"))); + assert!(text.iter().any(|line| line.contains("+1 -1"))); assert!( text.iter().any(|line| line.contains(" + new")), "added line should carry + gutter: {text:?}" @@ -478,4 +542,68 @@ diff --git a/src/a.rs b/src/a.rs assert_eq!(lines.join(""), text); } + + #[test] + fn render_diff_compact_skips_summary_and_file_headers() { + // The compact renderer is what the approval popup uses, so summary + // / `diff --git` / `--- +++` rows must be dropped to free vertical + // room for actual code. Hunk header + line-numbered code rows stay. + let diff = "\ +diff --git a/src/a.rs b/src/a.rs +--- a/src/a.rs ++++ b/src/a.rs +@@ -1,2 +1,3 @@ + line ++new +-old +"; + + let rendered = render_diff_compact(diff, 80); + let text: Vec = rendered.iter().map(line_text).collect(); + let joined = text.join("\n"); + assert!( + !joined.contains("summary:"), + "compact must skip summary: {joined}" + ); + assert!( + !joined.contains("diff --git"), + "compact must skip file headers: {joined}", + ); + assert!( + !joined.contains("--- a/src/a.rs"), + "compact must skip --- header: {joined}", + ); + assert!( + text.iter().any(|line| line.contains("@@ -1,2 +1,3 @@")), + "hunk header should remain: {text:?}", + ); + assert!( + text.iter().any(|line| line.contains(" + new")), + "added line should carry + gutter: {text:?}", + ); + assert!( + text.iter().any(|line| line.contains(" - old")), + "deleted line should carry - gutter: {text:?}", + ); + } + + #[test] + fn render_diff_preserves_indented_context_lines() { + let diff = "\ +diff --git a/src/a.rs b/src/a.rs +--- a/src/a.rs ++++ b/src/a.rs +@@ -1,2 +1,2 @@ + fn main() { + println!(\"hello\"); +"; + let rendered = render_diff_compact(diff, 80); + let text = rendered + .iter() + .map(line_text) + .collect::>() + .join("\n"); + assert!(text.contains(" fn main() {"), "{text}"); + assert!(text.contains(" println!"), "{text}"); + } } diff --git a/crates/tui/src/tui/pager.rs b/crates/tui/src/tui/pager.rs index a67339c75..dd434bf69 100644 --- a/crates/tui/src/tui/pager.rs +++ b/crates/tui/src/tui/pager.rs @@ -23,7 +23,7 @@ use ratatui::{ text::{Line, Span}, widgets::{Block, Borders, Clear, Padding, Paragraph, Widget, Wrap}, }; -use unicode_width::{UnicodeWidthChar, UnicodeWidthStr}; +use unicode_width::UnicodeWidthChar; use crate::palette; use crate::tui::views::{ModalKind, ModalView, ViewAction, ViewEvent}; @@ -70,7 +70,7 @@ impl PagerView { pub fn from_text(title: impl Into, text: &str, width: u16) -> Self { let mut lines = Vec::new(); for raw in text.lines() { - for wrapped in wrap_text(raw, width.max(1) as usize) { + for wrapped in wrap_text_preserving_spaces(raw, width.max(1) as usize) { lines.push(Line::from(Span::raw(wrapped))); } if raw.is_empty() { @@ -487,41 +487,57 @@ fn line_to_string(line: &Line<'static>) -> String { .collect::() } -fn wrap_text(text: &str, width: usize) -> Vec { +fn wrap_text_preserving_spaces(text: &str, width: usize) -> Vec { if width == 0 { return vec![text.to_string()]; } + if text.is_empty() { + return vec![String::new()]; + } + + let leading_indent = text + .chars() + .take_while(|ch| ch.is_whitespace()) + .collect::(); + let indent_width = display_width(&leading_indent); + let continuation_indent = if indent_width < width { + leading_indent + } else { + String::new() + }; + let mut lines = Vec::new(); let mut current = String::new(); let mut current_width = 0usize; - for word in text.split_whitespace() { - let word_width = word.width(); - if word_width > width { - if !current.is_empty() { + for ch in text.chars() { + let char_width = ch.width().unwrap_or(1); + if current_width + char_width > width && current_width > 0 { + if let Some((break_idx, break_len)) = last_word_break(¤t) { + let line = current[..break_idx].trim_end().to_string(); + let mut rest = current[break_idx + break_len..].trim_start().to_string(); + lines.push(line); + current.clear(); + current_width = 0; + + let rest_width = display_width(&rest); + if !rest.is_empty() + && !continuation_indent.is_empty() + && display_width(&continuation_indent) + rest_width <= width + { + current.push_str(&continuation_indent); + current_width += display_width(&continuation_indent); + } + current.push_str(&rest); + current_width += rest_width; + rest.clear(); + } else { lines.push(std::mem::take(&mut current)); current_width = 0; } - push_word_breaking_chars(word, width, &mut current, &mut current_width, &mut lines); - continue; - } - let additional = if current.is_empty() { - word_width - } else { - word_width + 1 - }; - if current_width + additional > width && !current.is_empty() { - lines.push(current); - current = word.to_string(); - current_width = word_width; - } else { - if !current.is_empty() { - current.push(' '); - current_width += 1; - } - current.push_str(word); - current_width += word_width; } + current.push(ch); + current_width += char_width; } if current.is_empty() { @@ -533,22 +549,23 @@ fn wrap_text(text: &str, width: usize) -> Vec { lines } -fn push_word_breaking_chars( - word: &str, - width: usize, - current: &mut String, - current_width: &mut usize, - lines: &mut Vec, -) { - for ch in word.chars() { - let char_width = ch.width().unwrap_or(1); - if *current_width + char_width > width && *current_width > 0 { - lines.push(std::mem::take(current)); - *current_width = 0; +fn last_word_break(text: &str) -> Option<(usize, usize)> { + let mut seen_non_ws = false; + let mut last = None; + for (idx, ch) in text.char_indices() { + if ch.is_whitespace() { + if seen_non_ws { + last = Some((idx, ch.len_utf8())); + } + } else { + seen_non_ws = true; } - current.push(ch); - *current_width += char_width; } + last +} + +fn display_width(text: &str) -> usize { + text.chars().map(|ch| ch.width().unwrap_or(1)).sum() } #[cfg(test)] @@ -814,6 +831,13 @@ mod tests { assert_eq!(p.search_input, "c"); } + #[test] + fn from_text_preserves_leading_indent() { + let p = PagerView::from_text("T", " indented\n more", 40); + assert!(p.body_text().contains(" indented")); + assert!(p.body_text().contains(" more")); + } + #[test] fn footer_hint_is_rendered_in_buffer() { let p = make_pager(5); @@ -1003,13 +1027,31 @@ mod tests { assert_eq!(p.scroll, bottom); } + #[test] + fn wrap_text_prefers_word_boundaries_for_english() { + let lines = wrap_text_preserving_spaces("alpha beta gamma delta", 12); + + assert_eq!(lines, vec!["alpha beta", "gamma delta"]); + } + + #[test] + fn wrap_text_keeps_continuation_indent() { + let lines = wrap_text_preserving_spaces(" alpha beta gamma", 14); + + assert_eq!(lines, vec![" alpha", " beta gamma"]); + } + #[test] fn wrap_text_breaks_overlong_cjk_runs() { let text = "这是一个非常长的中文字符串".repeat(10); - let lines = wrap_text(&text, 16); + let lines = wrap_text_preserving_spaces(&text, 16); for line in &lines { - assert!(line.width() <= 16, "line {line:?} exceeds width 16"); + let width = line + .chars() + .map(|ch| ch.width().unwrap_or(1)) + .sum::(); + assert!(width <= 16, "line {line:?} exceeds width 16"); } assert_eq!(lines.join(""), text); diff --git a/crates/tui/src/tui/ui.rs b/crates/tui/src/tui/ui.rs index ef3c2a386..5bef8352b 100644 --- a/crates/tui/src/tui/ui.rs +++ b/crates/tui/src/tui/ui.rs @@ -1771,6 +1771,7 @@ async fn run_event_loop( EngineEvent::ApprovalRequired { id, tool_name, + input, description, approval_key, approval_grouping_key, @@ -1816,24 +1817,18 @@ async fn run_event_loop( app.status_message = Some(format!("Blocked tool '{tool_name}' (approval_mode=never)")); } else { - let tool_input = app - .pending_tool_uses - .iter() - .find(|(tool_id, _, _)| tool_id == &id) - .map(|(_, _, input)| input.clone()) - .unwrap_or_else(|| serde_json::json!({})); - if tool_name == "apply_patch" { - maybe_add_patch_preview(app, &tool_input); + maybe_add_patch_preview(app, &input); } // Create approval request and show overlay - let request = ApprovalRequest::new( + let request = ApprovalRequest::new_with_workspace( &id, &tool_name, &description, - &tool_input, + &input, &approval_key, + Some(app.workspace.display().to_string()), ); log_sensitive_event( "tool.approval.prompted", @@ -4368,6 +4363,10 @@ fn open_text_pager(app: &mut App, title: String, content: String) { )); } +fn open_styled_pager(app: &mut App, title: String, lines: Vec>) { + app.view_stack.push(PagerView::new(title, lines)); +} + pub(crate) fn open_context_inspector(app: &mut App) { let width = app .viewport @@ -5757,6 +5756,9 @@ async fn handle_view_events( ViewEvent::OpenTextPager { title, content } => { open_text_pager(app, title, content); } + ViewEvent::OpenStyledPager { title, lines } => { + open_styled_pager(app, title, lines); + } ViewEvent::CopyToClipboard { text, label } => { if text.is_empty() { app.status_message = Some(format!("{label} is empty")); @@ -7425,6 +7427,8 @@ fn spillover_pager_section(app: &App, cell_index: usize) -> Option { } pub(crate) fn open_details_pager_for_cell(app: &mut App, cell_index: usize) -> bool { + use ratatui::style::Modifier; + use ratatui::text::{Line, Span}; if let Some(detail) = app.tool_detail_record_for_cell(cell_index) { let input = serde_json::to_string_pretty(&detail.input) .unwrap_or_else(|_| detail.input.to_string()); @@ -7432,6 +7436,12 @@ pub(crate) fn open_details_pager_for_cell(app: &mut App, cell_index: usize) -> b "(not available)".to_string(), std::string::ToString::to_string, ); + let workspace_str = app.workspace.to_string_lossy().to_string(); + let preview = crate::tui::approval::build_diff_preview( + &detail.tool_name, + &detail.input, + Some(workspace_str.as_str()), + ); // #500: when the tool result was spilled to disk, fold the full // file content into the pager body so the user can see what was @@ -7440,28 +7450,63 @@ pub(crate) fn open_details_pager_for_cell(app: &mut App, cell_index: usize) -> b // model received against the full payload. let spillover_section = spillover_pager_section(app, cell_index); - let content = if let Some(section) = spillover_section { - format!( - "Tool ID: {}\nTool: {}\n\nInput:\n{}\n\nOutput:\n{}\n\n{}", - detail.tool_id, detail.tool_name, input, output, section - ) - } else { - format!( - "Tool ID: {}\nTool: {}\n\nInput:\n{}\n\nOutput:\n{}", - detail.tool_id, detail.tool_name, input, output - ) - }; - let width = app .viewport .last_transcript_area .map(|area| area.width) .unwrap_or(80); - app.view_stack.push(PagerView::from_text( - format!("Tool: {}", detail.tool_name), - &content, - width.saturating_sub(2), - )); + // Subtract pager border + padding so wrapping in `render_diff` + // matches what the user actually sees inside the popup. + let diff_width = width.saturating_sub(4).max(20); + + let label_style = Style::default() + .fg(palette::DEEPSEEK_SKY) + .add_modifier(Modifier::BOLD); + let muted_style = Style::default().fg(palette::TEXT_MUTED); + let push_plain = |lines: &mut Vec>, text: &str| { + for raw in text.split('\n') { + lines.push(Line::from(Span::raw(raw.to_string()))); + } + }; + + let mut lines: Vec> = Vec::new(); + lines.push(Line::from(vec![ + Span::styled("Tool ID: ", muted_style), + Span::raw(detail.tool_id.clone()), + ])); + lines.push(Line::from(vec![ + Span::styled("Tool: ", muted_style), + Span::raw(detail.tool_name.clone()), + ])); + lines.push(Line::from("")); + + if let Some(preview) = preview.as_ref() { + lines.push(Line::from(Span::styled("Changes:", label_style))); + let diff_text = preview.diff_text(); + if diff_text.is_empty() { + lines.push(Line::from(Span::styled( + " (no textual changes — content matches current file)".to_string(), + muted_style, + ))); + } else { + lines.extend(crate::tui::diff_render::render_diff(diff_text, diff_width)); + } + lines.push(Line::from("")); + } + + lines.push(Line::from(Span::styled("Input:", label_style))); + push_plain(&mut lines, &input); + lines.push(Line::from("")); + lines.push(Line::from(Span::styled("Output:", label_style))); + push_plain(&mut lines, &output); + + if let Some(section) = spillover_section { + lines.push(Line::from("")); + push_plain(&mut lines, §ion); + } + + app.view_stack + .push(PagerView::new(format!("Tool: {}", detail.tool_name), lines)); return true; } diff --git a/crates/tui/src/tui/views/mod.rs b/crates/tui/src/tui/views/mod.rs index 7c3bd73d6..76a48e2fa 100644 --- a/crates/tui/src/tui/views/mod.rs +++ b/crates/tui/src/tui/views/mod.rs @@ -1,5 +1,5 @@ use crossterm::event::{KeyCode, KeyEvent, KeyModifiers, MouseButton, MouseEvent, MouseEventKind}; -use ratatui::{buffer::Buffer, layout::Rect}; +use ratatui::{buffer::Buffer, layout::Rect, text::Line}; use std::cell::{Cell, RefCell}; use std::fmt; @@ -87,6 +87,10 @@ pub enum ViewEvent { title: String, content: String, }, + OpenStyledPager { + title: String, + lines: Vec>, + }, ApprovalDecision { tool_id: String, tool_name: String, diff --git a/crates/tui/src/tui/widgets/mod.rs b/crates/tui/src/tui/widgets/mod.rs index 226ad72a3..95db1e0a2 100644 --- a/crates/tui/src/tui/widgets/mod.rs +++ b/crates/tui/src/tui/widgets/mod.rs @@ -26,7 +26,8 @@ use crate::localization::Locale; use crate::palette; use crate::tui::app::{App, AppMode, ComposerDensity, VimMode}; use crate::tui::approval::{ - ApprovalRequest, ApprovalView, ElevationOption, ElevationRequest, RiskLevel, ToolCategory, + ApprovalDiffPreview, ApprovalRequest, ApprovalView, ElevationOption, ElevationRequest, + RenderedDiffPanel, RiskLevel, ToolCategory, }; use crate::tui::history::HistoryCell; use crate::tui::scrolling::TranscriptLineMeta; @@ -1049,7 +1050,7 @@ const APPROVAL_CARD_HORIZONTAL_PAD: u16 = 6; const APPROVAL_CARD_VERTICAL_PAD: u16 = 2; /// Minimum card height — anything tighter and the destructive variant's /// confirmation banner overlaps the option list. -const APPROVAL_CARD_MIN_HEIGHT: u16 = 18; +const APPROVAL_CARD_MIN_HEIGHT: u16 = 16; /// Minimum card width — anything tighter makes approval copy wrap too /// aggressively on small terminals. const APPROVAL_CARD_MIN_WIDTH: u16 = 40; @@ -1098,9 +1099,10 @@ impl Renderable for ApprovalWidget<'_> { let palette_colors = approval_palette(risk); let mut lines: Vec> = Vec::with_capacity(20); - // Header: stakes badge + tool identifier. The badge is the - // first thing the eye lands on. + // Header: stakes badge + plain category. Keep the tool id out of + // the first read path so the command/path remains the focus. lines.push(Line::from("")); + let (cat_label, cat_color) = category_label_for(self.request.category, locale); lines.push(Line::from(vec![ Span::raw(" "), Span::styled( @@ -1111,20 +1113,6 @@ impl Renderable for ApprovalWidget<'_> { .add_modifier(Modifier::BOLD), ), Span::raw(" "), - Span::styled( - self.request.tool_name.clone(), - Style::default() - .fg(palette::DEEPSEEK_SKY) - .add_modifier(Modifier::BOLD), - ), - ])); - - // Category line — English remains the baseline while localized - // sessions get the same risk category in their UI language. - let (cat_label, cat_color) = category_label_for(self.request.category, locale); - lines.push(Line::from(vec![ - Span::raw(" "), - Span::styled(label_type(locale), Style::default().fg(palette::TEXT_HINT)), Span::styled( cat_label, Style::default().fg(cat_color).add_modifier(Modifier::BOLD), @@ -1132,48 +1120,89 @@ impl Renderable for ApprovalWidget<'_> { ])); lines.push(Line::from("")); - // About + impacts. Impact lines are the load-bearing content; - // they tell the user what will happen. - lines.push(Line::from(vec![ - Span::raw(" "), - Span::styled(label_about(locale), Style::default().fg(palette::TEXT_HINT)), - Span::styled( - self.request.description_for_locale(locale), - Style::default().fg(palette::TEXT_BODY), - ), - ])); - for impact in self.request.impacts_for_locale(locale).into_iter().take(4) { + // Prominent key details (command, path, target) are the default + // body. Full JSON stays behind `v`. + let details = self.request.prominent_detail_items(locale); + if details.is_empty() { lines.push(Line::from(vec![ Span::raw(" "), + Span::styled(label_tool(locale), Style::default().fg(palette::TEXT_HINT)), Span::styled( - label_impact(locale), - Style::default().fg(palette::TEXT_HINT), + self.request.tool_name.clone(), + Style::default() + .fg(palette::TEXT_BODY) + .add_modifier(Modifier::BOLD), ), - Span::styled(impact, Style::default().fg(palette::TEXT_BODY)), ])); + } else { + for detail in &details { + if self.request.category == ToolCategory::Shell + && matches!(detail.label.as_str(), "Command" | "命令") + && let Some(shell_lines) = detail.shell_lines.as_deref() + { + push_shell_command_lines(&mut lines, &detail.label, shell_lines); + } else { + push_detail_line(&mut lines, &detail.label, &detail.value); + } + } } - lines.push(Line::from("")); - let params_str = self.request.params_display(); - let params_width = card_area.width.saturating_sub(14) as usize; - let params_truncated = - crate::utils::truncate_with_ellipsis(¶ms_str, params_width.max(20), "..."); - lines.push(Line::from(vec![ - Span::raw(" "), - Span::styled( - label_params(locale), - Style::default().fg(palette::TEXT_HINT), - ), - Span::styled( - params_truncated, - Style::default().fg(palette::TEXT_SECONDARY), - ), - ])); + let options = approval_options_for(self.request, risk, locale); + let pending = self.view.pending_confirm(); - lines.push(Line::from("")); + if let Some(preview) = self.request.diff_preview() { + let diff_width = card_area.width.saturating_sub(4).max(20); + let RenderedDiffPanel { + header, + lines: body_lines, + } = self + .view + .cached_diff_panel(diff_width, locale) + .unwrap_or_else(|| { + let (header, lines) = build_diff_panel(preview, diff_width, locale); + let panel = RenderedDiffPanel { header, lines }; + self.view + .set_cached_diff_panel(diff_width, locale, panel.clone()); + panel + }); + let body_height = card_area.height.saturating_sub(4) as usize; + let footer_rows = match (risk, pending) { + (RiskLevel::Destructive, Some(_)) if !details.is_empty() => 2, + _ => 1, + }; + // Keep the action rows and confirmation copy visible while the + // diff itself scrolls inside the approval card. + let reserved_after_diff = 1 + 4 + 1 + footer_rows; + let available = body_height + .saturating_sub(lines.len()) + .saturating_sub(reserved_after_diff) + .saturating_sub(2); + let visible = available.min(body_lines.len()); + let scroll = self.view.set_diff_metrics(body_lines.len(), visible); - let options = approval_options_for(risk, locale); - let pending = self.view.pending_confirm(); + lines.push(Line::from("")); + if visible > 0 { + let end = scroll.saturating_add(visible).min(body_lines.len()); + let more_above = scroll > 0; + let more_below = end < body_lines.len(); + lines.push(Line::from(Span::styled( + diff_header_with_scroll(&header, more_above, more_below, locale), + Style::default().fg(palette::TEXT_HINT), + ))); + for line in body_lines.into_iter().skip(scroll).take(visible) { + lines.push(line); + } + } else { + lines.push(Line::from(Span::styled( + format!(" {header} · {}", diff_hidden_hint(locale)), + Style::default().fg(palette::TEXT_HINT), + ))); + } + } else { + self.view.set_diff_metrics(0, 0); + } + + lines.push(Line::from("")); for (i, opt) in options.iter().enumerate() { let is_selected = i == self.view.selected(); @@ -1200,7 +1229,7 @@ impl Renderable for ApprovalWidget<'_> { .fg(palette_colors.shortcut) .add_modifier(Modifier::BOLD), ), - Span::styled(opt.label.to_string(), row_style.fg(label_color)), + Span::styled(opt.label.clone(), row_style.fg(label_color)), ]; if staged { spans.push(Span::raw(" ")); @@ -1246,6 +1275,23 @@ impl Renderable for ApprovalWidget<'_> { } _ => "Enter", }; + // Show what is being confirmed so the user never loses + // context between the selection and the commit step. + if let Some(detail) = details.first() { + lines.push(Line::from(vec![ + Span::raw(" "), + Span::styled( + confirm_label(locale, &detail.label), + Style::default().fg(palette::TEXT_BODY), + ), + Span::styled( + detail.value.clone(), + Style::default() + .fg(palette::TEXT_BODY) + .add_modifier(Modifier::BOLD), + ), + ])); + } lines.push(Line::from(vec![ Span::raw(" "), Span::styled( @@ -1419,31 +1465,25 @@ fn approval_word(locale: Locale) -> &'static str { } } -fn label_type(locale: Locale) -> &'static str { - match locale { - Locale::ZhHans => "类型：", - _ => "Type: ", - } -} - -fn label_about(locale: Locale) -> &'static str { - match locale { - Locale::ZhHans => "说明：", - _ => "About: ", - } -} - -fn label_impact(locale: Locale) -> &'static str { +fn label_tool(locale: Locale) -> &'static str { match locale { - Locale::ZhHans => "影响：", - _ => "Impact: ", + Locale::ZhHans => "工具 ", + _ => "Tool ", } } -fn label_params(locale: Locale) -> &'static str { +fn confirm_label(locale: Locale, detail_label: &str) -> String { match locale { - Locale::ZhHans => "参数：", - _ => "Params: ", + Locale::ZhHans => match detail_label { + "Command" | "命令" => "确认命令：".to_string(), + "File" | "文件" => "确认文件：".to_string(), + "Dir" | "目录" => "确认目录：".to_string(), + "Target" | "目标" => "确认目标：".to_string(), + "Path" | "路径" => "确认路径：".to_string(), + "Input" | "输入" => "确认输入：".to_string(), + _ => format!("确认{detail_label}："), + }, + _ => format!("Confirm {}: ", detail_label.to_ascii_lowercase()), } } @@ -1467,8 +1507,259 @@ fn single_key_value(_locale: Locale) -> &'static str { fn footer_controls(locale: Locale) -> &'static str { match locale { - Locale::ZhHans => " · v：完整参数 · Esc：终止", - _ => " · v: full params · Esc: abort", + Locale::ZhHans => " · v：完整 diff · Esc：终止", + _ => " · v: full details · Esc: abort", + } +} + +/// Build the rendered body + header for the approval popup diff area. +/// +/// The popup uses the compact renderer (no summary / `--- +++` lines) so a +/// 10-row preview window shows a few real lines of code instead of just +/// metadata. The header summarises the variant — `+N -M` for normal diffs, +/// a "new file" or "no change" hint for the degenerate cases — so the user +/// always knows what's behind the preview window even when it's empty. +/// Build the rendered body + header for the approval popup diff area. +/// +/// The popup uses the compact renderer (no summary / `--- +++` lines) so a +/// 10-row preview window shows a few real lines of code instead of just +/// metadata. The header summarises the variant — `+N -M` for normal diffs, +/// a "new file" or "no change" hint for the degenerate cases — so the user +/// always knows what's behind the preview window even when it's empty. +/// +/// Body lines are indented by two spaces to line up with the rest of the +/// popup body (the popup margin) so the hunk header and code rows visually +/// align with the `File` / option rows above and below. +fn build_diff_panel( + preview: &ApprovalDiffPreview, + width: u16, + locale: Locale, +) -> (String, Vec>) { + use crate::tui::diff_render::render_diff_compact; + // Reserve 2 columns for the indent we prepend below so the renderer's + // wrapping decisions agree with the visible width. + let body_width = width.saturating_sub(2).max(20); + let indent = || Span::raw(" "); + let indent_lines = |mut lines: Vec>| -> Vec> { + for line in &mut lines { + line.spans.insert(0, indent()); + } + lines + }; + match preview { + ApprovalDiffPreview::Diff { + text, + added, + deleted, + } => { + let lines = indent_lines(render_diff_compact(text, body_width)); + let header = format!("{} +{} -{}", diff_preview_word(locale), added, deleted); + (header, lines) + } + ApprovalDiffPreview::NewFile { path: _, content } => { + // Render the proposed content directly as added lines so the + // user sees the body with the standard `+` gutter + new-side + // line numbers. No synthetic hunk header — the panel header + // already says "New file +N", and the hunk row would just + // duplicate metadata while costing one of the few rows we have. + let body = content + .lines() + .enumerate() + .map(|(idx, line)| { + // Reuse the diff renderer by feeding a synthetic line + // wearing the `+` marker, but the renderer doesn't see + // a hunk header so we build the styled line by hand + // to keep colour + line numbers consistent. + let prefix = format!("{:>4} {:>4} + ", "", idx + 1); + Line::from(vec![ + Span::raw(" "), + Span::styled(prefix, Style::default().fg(palette::TEXT_MUTED)), + Span::styled( + line.to_string(), + Style::default() + .fg(palette::DIFF_ADDED) + .bg(palette::DIFF_ADDED_BG), + ), + ]) + }) + .collect::>(); + let header = format!("{} +{}", new_file_word(locale), content.lines().count()); + (header, body) + } + ApprovalDiffPreview::NoChange { path: _ } => { + let msg = no_change_text(locale); + let line = Line::from(Span::styled( + msg.to_string(), + Style::default().fg(palette::TEXT_MUTED), + )); + (no_change_word(locale).to_string(), vec![line]) + } + ApprovalDiffPreview::SkippedLargeFile { size, limit, .. } => { + let line = Line::from(Span::styled( + skipped_large_file_text(*size, *limit, locale), + Style::default().fg(palette::TEXT_MUTED), + )); + (preview_skipped_word(locale).to_string(), vec![line]) + } + ApprovalDiffPreview::MissingMatch { + path: _, + text, + match_count: _, + } => { + let mut lines = vec![Line::from(Span::styled( + missing_match_warning(locale).to_string(), + Style::default() + .fg(palette::STATUS_WARNING) + .add_modifier(Modifier::BOLD), + ))]; + lines.extend(indent_lines(render_diff_compact(text, body_width))); + (missing_match_header(locale).to_string(), lines) + } + } +} + +fn diff_header_with_scroll( + header: &str, + more_above: bool, + more_below: bool, + locale: Locale, +) -> String { + let mut parts: Vec = vec![format!(" {header}")]; + if more_above { + parts.push(more_above_hint(locale).to_string()); + } + if more_below { + parts.push(more_below_hint(locale).to_string()); + } + parts.push(full_details_hint(locale).to_string()); + parts.join(" · ") +} + +fn diff_preview_word(locale: Locale) -> &'static str { + match locale { + Locale::ZhHans => "差异预览", + _ => "Diff preview", + } +} + +fn new_file_word(locale: Locale) -> &'static str { + match locale { + Locale::ZhHans => "新文件", + _ => "New file", + } +} + +fn no_change_word(locale: Locale) -> &'static str { + match locale { + Locale::ZhHans => "无变化", + _ => "No change", + } +} + +fn preview_skipped_word(locale: Locale) -> &'static str { + match locale { + Locale::ZhHans => "预览已跳过", + _ => "Preview skipped", + } +} + +fn skipped_large_file_text(size: u64, limit: u64, locale: Locale) -> String { + match locale { + Locale::ZhHans => format!("文件过大，已跳过差异预览（{size} bytes，限制 {limit} bytes）"), + _ => format!( + "file is too large for an inline diff preview ({size} bytes, limit {limit} bytes)" + ), + } +} + +fn no_change_text(locale: Locale) -> &'static str { + match locale { + Locale::ZhHans => " 内容与当前文件一致，无需写入。", + _ => " Content matches the current file — nothing to write.", + } +} + +fn missing_match_warning(locale: Locale) -> &'static str { + match locale { + Locale::ZhHans => " ⚠ 文件中未匹配到 search，下面是 search→replace 兜底视图。", + _ => " ⚠ Search string not found in file — fallback search→replace below.", + } +} + +fn missing_match_header(locale: Locale) -> &'static str { + match locale { + Locale::ZhHans => "未匹配（兜底视图）", + _ => "No match (fallback)", + } +} + +fn more_above_hint(locale: Locale) -> &'static str { + match locale { + Locale::ZhHans => "↑ 上方还有", + _ => "↑ more above", + } +} + +fn more_below_hint(locale: Locale) -> &'static str { + match locale { + Locale::ZhHans => "↓ 下方还有", + _ => "↓ more below", + } +} + +fn full_details_hint(locale: Locale) -> &'static str { + match locale { + Locale::ZhHans => "v 详情", + _ => "v details", + } +} + +fn push_detail_line(lines: &mut Vec>, label: &str, value: &str) { + lines.push(Line::from(vec![ + Span::raw(" "), + Span::styled( + format!("{label:<7} "), + Style::default() + .fg(palette::DEEPSEEK_SKY) + .add_modifier(Modifier::BOLD), + ), + Span::styled( + value.to_string(), + Style::default() + .fg(palette::TEXT_BODY) + .add_modifier(Modifier::BOLD), + ), + ])); +} + +fn push_shell_command_lines(lines: &mut Vec>, label: &str, command_lines: &[String]) { + lines.push(Line::from(vec![ + Span::raw(" "), + Span::styled( + format!("{label:<7} "), + Style::default() + .fg(palette::DEEPSEEK_SKY) + .add_modifier(Modifier::BOLD), + ), + ])); + + for line in command_lines { + lines.push(Line::from(vec![ + Span::raw(" "), + Span::styled( + line.clone(), + Style::default() + .fg(palette::TEXT_BODY) + .add_modifier(Modifier::BOLD), + ), + ])); + } +} + +fn diff_hidden_hint(locale: Locale) -> &'static str { + match locale { + Locale::ZhHans => "终端高度不足，按 v 查看", + _ => "preview hidden — press v", } } @@ -1516,36 +1807,40 @@ fn two_key_value(locale: Locale) -> &'static str { struct ApprovalOptionRow { option: crate::tui::approval::ApprovalOption, - label: &'static str, + label: String, key_hint: &'static str, dangerous: bool, } -fn approval_options_for(risk: RiskLevel, locale: Locale) -> [ApprovalOptionRow; 4] { +fn approval_options_for( + request: &ApprovalRequest, + risk: RiskLevel, + locale: Locale, +) -> [ApprovalOptionRow; 4] { use crate::tui::approval::ApprovalOption as O; let dangerous = matches!(risk, RiskLevel::Destructive); [ ApprovalOptionRow { option: O::ApproveOnce, - label: option_approve_once(locale), + label: option_approve_once(locale).to_string(), key_hint: "1 / y", dangerous, }, ApprovalOptionRow { option: O::ApproveAlways, - label: option_approve_always(locale), + label: option_approve_always(request.category, locale), key_hint: "2 / a", dangerous, }, ApprovalOptionRow { option: O::Deny, - label: option_deny(locale), + label: option_deny(locale).to_string(), key_hint: "3 / d / n", dangerous: false, }, ApprovalOptionRow { option: O::Abort, - label: option_abort(locale), + label: option_abort(locale).to_string(), key_hint: "Esc", dangerous: false, }, @@ -1559,10 +1854,14 @@ fn option_approve_once(locale: Locale) -> &'static str { } } -fn option_approve_always(locale: Locale) -> &'static str { - match locale { - Locale::ZhHans => "本会话同类自动批准", - _ => "Approve always for this kind", +fn option_approve_always(category: ToolCategory, locale: Locale) -> String { + match (locale, category) { + (Locale::ZhHans, ToolCategory::Shell) => "本会话自动批准 Shell 命令".to_string(), + (Locale::ZhHans, ToolCategory::FileWrite) => "本会话自动批准文件写入".to_string(), + (Locale::ZhHans, _) => "本会话同类自动批准".to_string(), + (_, ToolCategory::Shell) => "Approve shell commands this session".to_string(), + (_, ToolCategory::FileWrite) => "Approve file writes this session".to_string(), + _ => "Approve this kind this session".to_string(), } } @@ -3349,6 +3648,96 @@ mod tests { } } + #[test] + fn approval_shell_command_splits_long_command_into_multiple_lines() { + let request = crate::tui::approval::ApprovalRequest::new( + "approval-1", + "exec_shell", + "Run shell command", + &serde_json::json!({ + "command": "printf 'a b c' && cat /tmp/x | sed -n '1,5p' > out.txt", + }), + "exec_shell:printf", + ); + let view = crate::tui::approval::ApprovalView::new(request.clone()); + let widget = ApprovalWidget::new(&request, &view); + let area = Rect::new(0, 0, 120, 24); + let mut buf = Buffer::empty(area); + widget.render(area, &mut buf); + + let mut body = String::new(); + for y in 0..area.height { + for x in 0..area.width { + body.push_str(buf[(x, y)].symbol()); + } + body.push('\n'); + } + assert!(body.contains("Command")); + assert!(body.contains("printf 'a b c'")); + assert!(body.contains("cat /tmp/x")); + assert!(body.contains("sed -n '1,5p'")); + } + + #[test] + fn approval_shell_command_detects_printf_write_file_preview() { + let request = crate::tui::approval::ApprovalRequest::new( + "approval-1", + "exec_shell", + "Run shell command", + &serde_json::json!({ + "command": "printf '%s\\n' \"line one\" \"line two\" > /tmp/out.txt", + }), + "exec_shell:printf", + ); + let view = crate::tui::approval::ApprovalView::new(request.clone()); + let widget = ApprovalWidget::new(&request, &view); + let area = Rect::new(0, 0, 120, 24); + let mut buf = Buffer::empty(area); + widget.render(area, &mut buf); + + let mut body = String::new(); + for y in 0..area.height { + for x in 0..area.width { + body.push_str(buf[(x, y)].symbol()); + } + body.push('\n'); + } + assert!(body.contains("Write file via printf"), "{body}"); + assert!(body.contains("Target: /tmp/out.txt"), "{body}"); + assert!(body.contains("Lines: 2"), "{body}"); + assert!(body.contains("1 line one"), "{body}"); + assert!(body.contains("2 line two"), "{body}"); + } + + #[test] + fn approval_shell_command_keeps_compound_printf_command_visible() { + let request = crate::tui::approval::ApprovalRequest::new( + "approval-1", + "exec_shell", + "Run shell command", + &serde_json::json!({ + "command": "printf '%s\\n' \"line one\" > /tmp/out.txt && rm /tmp/other.txt", + }), + "exec_shell:printf", + ); + let view = crate::tui::approval::ApprovalView::new(request.clone()); + let widget = ApprovalWidget::new(&request, &view); + let area = Rect::new(0, 0, 120, 24); + let mut buf = Buffer::empty(area); + widget.render(area, &mut buf); + + let mut body = String::new(); + for y in 0..area.height { + for x in 0..area.width { + body.push_str(buf[(x, y)].symbol()); + } + body.push('\n'); + } + assert!(!body.contains("Write file via printf"), "{body}"); + assert!(body.contains("printf '%s"), "{body}"); + assert!(body.contains("rm /tmp/other.txt"), "{body}"); + } + /// Regression for issue #65: after `App::handle_resize`, the chat widget /// must produce a clean render at the new width — no stale wrapping, /// no panic, no content exceeding the requested width. Cycling through