diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ec610e1..c3907c4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -82,6 +82,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `Esc` while editing a queued follow-up restores the original queued message instead of cancelling the active turn or silently dropping the queued work (#2054). +- Approval prompts now render prominent command, directory, file, path, or + target rows before falling back to raw JSON params. Shell approvals preserve + long command tails, split common shell chains for review, and show compact + `printf > file` previews while keeping intent summaries visible (#1991, + #2269). - Sidebar hover details now use row-level metadata for truncated Work, Tasks, and Agents rows. Mouse hover opens a bordered, wrapping popover with the full underlying row text, long turn/agent ids, and current sub-agent progress @@ -134,7 +139,9 @@ tool-call transcript collapse/sidebar detail direction (#2738, #2734, #2692, #2694), and the HarnessPosture config model for provider/model posture (#2741, #2693), and **@h3c-hexin** for the tool-agent model inheritance and configured -`skills_dir` fixes (#2736, #2737). Thanks also to **@qiyuanlicn** for the +`skills_dir` fixes (#2736, #2737), **@AresNing** for the turn-end observer hook +work (#2578), and **@tdccccc** for the approval key-detail and shell-preview +work (#1991, #2269). Thanks also to **@qiyuanlicn** for the checkpoint/resume report that shaped the sub-agent recovery slice (#2029), **@bevis-wong** for the long-running shell/task liveness report (#1786), **@shuxiangxuebiancheng** for the third-party OpenAI-compatible path report diff --git a/README.md b/README.md index 2bfcf155..c633420b 100644 --- a/README.md +++ b/README.md @@ -720,6 +720,7 @@ Current and recurring contributors include: - **[yuanchenglu](https://github.com/yuanchenglu)** — Feishu per-chat model switching (#2149) - **[HUQIANTAO](https://github.com/HUQIANTAO)** — Xiaomi balance/status work, stalled-turn recovery, approval intent summaries, mobile smoke/QR support, Claude theme, and broad docs/test/CI coverage (#2257, #2267, #2283, #2384, #2385, #2389, #2403, #2440-#2458, #2460) - **[h3c-hexin](https://github.com/h3c-hexin)** — web-search URL decoding, prompt/instructions override hooks, sub-agent guidance, SSRF fake-IP trust configuration, and prompt-cache-friendly environment placement (#2245, #2311, #2313, #2314, #2354, #2355, #2356) +- **[tdccccc](https://github.com/tdccccc)** — approval prompt key-detail and shell-preview work harvested into the maintained approval path (#1991, #2269) - **[AresNing](https://github.com/AresNing)** — first-run guide, message-submit hook transform design, and turn-end observer hook work harvested into the maintained hooks path (#2278, #2318, #2434, #2578) - **[Implementist](https://github.com/Implementist)** — Volcengine Ark search provider and reliability hardening (#2426, #2429, #2439) - **[lihuan215](https://github.com/lihuan215)** — Unix socket hook sink design harvested into the opt-in hook event path (#2333, #2430) diff --git a/crates/tui/CHANGELOG.md b/crates/tui/CHANGELOG.md index 7ec610e1..c3907c4c 100644 --- a/crates/tui/CHANGELOG.md +++ b/crates/tui/CHANGELOG.md @@ -82,6 +82,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `Esc` while editing a queued follow-up restores the original queued message instead of cancelling the active turn or silently dropping the queued work (#2054). +- Approval prompts now render prominent command, directory, file, path, or + target rows before falling back to raw JSON params. Shell approvals preserve + long command tails, split common shell chains for review, and show compact + `printf > file` previews while keeping intent summaries visible (#1991, + #2269). - Sidebar hover details now use row-level metadata for truncated Work, Tasks, and Agents rows. Mouse hover opens a bordered, wrapping popover with the full underlying row text, long turn/agent ids, and current sub-agent progress @@ -134,7 +139,9 @@ tool-call transcript collapse/sidebar detail direction (#2738, #2734, #2692, #2694), and the HarnessPosture config model for provider/model posture (#2741, #2693), and **@h3c-hexin** for the tool-agent model inheritance and configured -`skills_dir` fixes (#2736, #2737). Thanks also to **@qiyuanlicn** for the +`skills_dir` fixes (#2736, #2737), **@AresNing** for the turn-end observer hook +work (#2578), and **@tdccccc** for the approval key-detail and shell-preview +work (#1991, #2269). Thanks also to **@qiyuanlicn** for the checkpoint/resume report that shaped the sub-agent recovery slice (#2029), **@bevis-wong** for the long-running shell/task liveness report (#1786), **@shuxiangxuebiancheng** for the third-party OpenAI-compatible path report diff --git a/crates/tui/src/tui/approval.rs b/crates/tui/src/tui/approval.rs index a8abe839..361cb751 100644 --- a/crates/tui/src/tui/approval.rs +++ b/crates/tui/src/tui/approval.rs @@ -140,6 +140,16 @@ pub struct ApprovalRequest { pub intent_summary: Option, } +/// Key approval details rendered prominently in the approval card. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ApprovalDetail { + pub label: String, + pub value: String, + /// Preformatted shell lines for commands that benefit from safe wrapping + /// or a compact write-file preview. `value` remains the original command. + pub shell_lines: Option>, +} + impl ApprovalRequest { #[cfg(test)] pub fn new( @@ -207,6 +217,18 @@ impl ApprovalRequest { _ => self.impacts.clone(), } } + + /// Extract the most important params for the approval card. + #[must_use] + pub fn prominent_detail_items(&self, locale: Locale) -> Vec { + build_prominent_details(self.category, &self.params) + .into_iter() + .map(|mut detail| { + detail.label = localize_detail_label(&detail.label, locale).to_string(); + detail + }) + .collect() + } } /// Get the category for a tool by name @@ -215,7 +237,16 @@ 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" + | "task_shell_wait" + | "exec_shell_wait" + | "exec_shell_interact" + | "exec_wait" + | "exec_interact" + ) { ToolCategory::Shell } else if name.starts_with("list_mcp_") || name.starts_with("read_mcp_") @@ -470,6 +501,287 @@ fn build_impact_summary_zh_hans( } } +fn build_prominent_details(category: ToolCategory, params: &Value) -> Vec { + let mut details = Vec::new(); + match category { + ToolCategory::Shell => { + if let Some(command) = param_text(params, &["command", "cmd"]) { + details.push(ApprovalDetail { + label: "Command".to_string(), + shell_lines: Some(format_shell_command_for_approval(&command)), + value: command, + }); + } + if let Some(workdir) = param_preview(params, &["workdir", "cwd"], 96) { + details.push(ApprovalDetail { + label: "Dir".to_string(), + value: workdir, + shell_lines: None, + }); + } + } + ToolCategory::FileWrite => { + if let Some(path) = param_preview(params, &["path", "target", "destination"], 200) { + details.push(ApprovalDetail { + label: "File".to_string(), + value: path, + shell_lines: None, + }); + } + } + ToolCategory::Safe => { + if let Some(path) = param_preview(params, &["path", "ref_id", "uri"], 200) { + details.push(ApprovalDetail { + label: "Path".to_string(), + value: path, + shell_lines: None, + }); + } + } + ToolCategory::Network => { + if let Some(target) = + param_preview(params, &["url", "q", "query", "location", "repo"], 200) + { + details.push(ApprovalDetail { + label: "Target".to_string(), + value: target, + shell_lines: None, + }); + } + } + ToolCategory::McpRead | ToolCategory::McpAction | ToolCategory::Unknown => { + if let Some(input) = param_preview( + params, + &["command", "cmd", "path", "url", "q", "query", "ref_id"], + 200, + ) { + details.push(ApprovalDetail { + label: "Input".to_string(), + value: input, + shell_lines: None, + }); + } + } + } + details +} + +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(text.clone()), + Value::Number(number) => return Some(number.to_string()), + Value::Bool(flag) => return Some(flag.to_string()), + other => return Some(other.to_string()), + } + } + + None +} + +fn localize_detail_label(label: &str, locale: Locale) -> &str { + match locale { + Locale::ZhHans => match label { + "Command" => "命令", + "Dir" => "目录", + "File" => "文件", + "Path" => "路径", + "Target" => "目标", + "Input" => "输入", + _ => label, + }, + _ => label, + } +} + +pub(crate) fn format_shell_command_for_approval(command: &str) -> Vec { + if let Some(preview) = parse_printf_write_file_command(command) { + return format_printf_write_file_preview(preview); + } + + let mut out = Vec::new(); + for raw_line in command.lines() { + split_shell_display_line(raw_line, &mut out); + } + if out.is_empty() && !command.trim().is_empty() { + out.push(command.trim().to_string()); + } + out +} + +fn split_shell_display_line(line: &str, out: &mut Vec) { + let mut quote: Option = None; + let mut escaped = false; + let mut current = String::new(); + let mut chars = line.chars().peekable(); + + while let Some(ch) = chars.next() { + if escaped { + current.push(ch); + escaped = false; + continue; + } + + if ch == '\\' { + current.push(ch); + escaped = true; + continue; + } + + if matches!(ch, '"' | '\'') { + if quote == Some(ch) { + quote = None; + } else if quote.is_none() { + quote = Some(ch); + } + current.push(ch); + continue; + } + + if quote.is_none() { + match ch { + '&' if chars.peek() == Some(&'&') => { + chars.next(); + push_shell_clause(out, &mut current, Some("&&")); + continue; + } + '|' if chars.peek() == Some(&'|') => { + chars.next(); + push_shell_clause(out, &mut current, Some("||")); + continue; + } + '|' => { + push_shell_clause(out, &mut current, Some("|")); + continue; + } + ';' => { + push_shell_clause(out, &mut current, Some(";")); + continue; + } + _ => {} + } + } + + current.push(ch); + } + + push_shell_clause(out, &mut current, None); +} + +fn push_shell_clause(out: &mut Vec, current: &mut String, operator: Option<&str>) { + let trimmed = current.trim(); + if trimmed.is_empty() { + if let Some(operator) = operator { + out.push(operator.to_string()); + } + } else if let Some(operator) = operator { + out.push(format!("{trimmed} {operator}")); + } else { + out.push(trimmed.to_string()); + } + current.clear(); +} + +#[derive(Debug, Clone, PartialEq, Eq)] +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.first()?.as_str() != "printf" { + return None; + } + let target_parts = shlex::split(after_redirect.trim())?; + if target_parts.len() != 1 { + return None; + } + let target = target_parts + .into_iter() + .next()? + .trim_matches(|ch| ch == '"' || ch == '\'') + .to_string(); + if target.is_empty() { + return None; + } + + let args = &tokens[1..]; + if args.is_empty() { + return None; + } + let values = if args.len() >= 2 && args[0].contains('%') { + &args[1..] + } else { + args + }; + let mut lines = Vec::new(); + for value in values { + let normalized = value.replace("\\n", "\n"); + for line in normalized.lines() { + lines.push(line.to_string()); + } + } + if lines.is_empty() { + lines.push(String::new()); + } + + Some(PrintfWriteFilePreview { target, lines }) +} + +fn format_printf_write_file_preview(preview: PrintfWriteFilePreview) -> Vec { + const MAX_PREVIEW_LINES: usize = 12; + let mut out = vec![format!("printf > {}", preview.target)]; + let total = preview.lines.len(); + for line in preview.lines.into_iter().take(MAX_PREVIEW_LINES) { + out.push(format!(" {line}")); + } + if total > MAX_PREVIEW_LINES { + out.push(format!(" ... (+{} more lines)", total - MAX_PREVIEW_LINES)); + } + out +} + +fn split_unquoted_redirect(command: &str) -> Option<(&str, &str)> { + let mut quote: Option = None; + let mut escaped = false; + for (idx, ch) in command.char_indices() { + if escaped { + escaped = false; + continue; + } + if ch == '\\' { + escaped = true; + continue; + } + if matches!(ch, '"' | '\'') { + if quote == Some(ch) { + quote = None; + } else if quote.is_none() { + quote = Some(ch); + } + continue; + } + if quote.is_none() && ch == '>' { + return Some((&command[..idx], &command[idx + ch.len_utf8()..])); + } + } + None +} + /// Indices into the option list shared by both variants. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ApprovalOption { @@ -970,6 +1282,15 @@ 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("task_shell_wait"), 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("exec_wait"), ToolCategory::Shell); + assert_eq!(get_tool_category("exec_interact"), ToolCategory::Shell); assert_eq!( get_tool_category("mcp_linear_save_issue"), ToolCategory::McpAction @@ -1126,6 +1447,66 @@ mod tests { ); } + #[test] + fn test_prominent_details_shell_does_not_truncate_long_command() { + let command = format!("printf '{}\\n' > /tmp/x && cat /tmp/x", "x".repeat(300)); + let request = ApprovalRequest::new( + "test-id", + "exec_shell", + "Run a shell command", + &json!({"command": command, "cwd": "/tmp/project"}), + "test_key", + ); + + let details = request.prominent_detail_items(Locale::En); + + assert_eq!(details[0].label, "Command"); + assert_eq!(details[0].value, command); + assert!( + details[0] + .shell_lines + .as_ref() + .is_some_and(|lines| lines.iter().any(|line| line.contains("cat /tmp/x"))), + "shell preview should preserve the dangerous tail of long commands" + ); + assert_eq!(details[1].label, "Dir"); + assert_eq!(details[1].value, "/tmp/project"); + } + + #[test] + fn test_prominent_details_file_write() { + let request = ApprovalRequest::new( + "test-id", + "write_file", + "Write a file to disk", + &json!({"path": "src/main.rs", "content": "fn main() {}"}), + "test_key", + ); + + let details = request.prominent_detail_items(Locale::En); + + assert_eq!(details[0].label, "File"); + assert_eq!(details[0].value, "src/main.rs"); + assert!(details[0].shell_lines.is_none()); + } + + #[test] + fn test_shell_formatter_preserves_logical_or_operator() { + let lines = format_shell_command_for_approval("cargo build || echo fallback"); + + assert_eq!(lines, vec!["cargo build ||", "echo fallback"]); + } + + #[test] + fn test_shell_formatter_detects_printf_write_file_preview() { + let lines = + format_shell_command_for_approval("printf '%s\\n' 'hello' 'world' > src/main.rs"); + + assert_eq!(lines[0], "printf > src/main.rs"); + assert!(lines.iter().any(|line| line.contains("hello"))); + assert!(lines.iter().any(|line| line.contains("world"))); + } + // ======================================================================== // ApprovalView Tests — Benign Variant (single-key approve) // ======================================================================== diff --git a/crates/tui/src/tui/widgets/mod.rs b/crates/tui/src/tui/widgets/mod.rs index 03acf4aa..55b8cff8 100644 --- a/crates/tui/src/tui/widgets/mod.rs +++ b/crates/tui/src/tui/widgets/mod.rs @@ -1270,21 +1270,35 @@ impl Renderable for ApprovalWidget<'_> { } 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 details = self.request.prominent_detail_items(locale); + if details.is_empty() { + 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), + ), + ])); + } else { + for detail in details.iter().take(4) { + 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("")); @@ -1507,6 +1521,43 @@ fn label_params(locale: Locale) -> &'static str { } } +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)), + ])); +} + +fn push_shell_command_lines(lines: &mut Vec>, label: &str, command_lines: &[String]) { + lines.push(Line::from(vec![ + Span::raw(" "), + Span::styled( + format!("{label}:"), + 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 footer_controls(locale: Locale) -> &'static str { match locale { Locale::ZhHans => " · v：完整参数 · Esc：终止", @@ -4025,6 +4076,65 @@ mod tests { ); } + #[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' 'alpha' 'beta' > src/generated.txt", + "cwd": "/tmp/project", + }), + "exec_shell:printf", + ); + let view = crate::tui::approval::ApprovalView::new(request.clone()); + let widget = ApprovalWidget::new(&request, &view); + let area = Rect::new(0, 0, 110, 32); + let mut buf = Buffer::empty(area); + + widget.render(area, &mut buf); + let rendered = buffer_text(&buf, area); + + assert!(rendered.contains("Command:"), "{rendered}"); + assert!( + rendered.contains("printf > src/generated.txt"), + "{rendered}" + ); + assert!(rendered.contains("alpha"), "{rendered}"); + assert!(rendered.contains("beta"), "{rendered}"); + assert!(rendered.contains("Dir"), "{rendered}"); + assert!(rendered.contains("/tmp/project"), "{rendered}"); + } + + #[test] + fn approval_intent_summary_still_renders_with_shell_details() { + let request = crate::tui::approval::ApprovalRequest::new_with_intent( + "approval-1", + "exec_shell", + "Run shell command", + &serde_json::json!({ + "command": "cargo build || echo fallback", + "cwd": "/tmp/project", + }), + "exec_shell:cargo", + Some("Need to verify the fallback build path before editing files."), + ); + let view = crate::tui::approval::ApprovalView::new(request.clone()); + let widget = ApprovalWidget::new(&request, &view); + let area = Rect::new(0, 0, 120, 34); + let mut buf = Buffer::empty(area); + + widget.render(area, &mut buf); + let rendered = buffer_text(&buf, area); + + assert!(rendered.contains("Intent:"), "{rendered}"); + assert!(rendered.contains("fallback build path"), "{rendered}"); + assert!(rendered.contains("Command:"), "{rendered}"); + assert!(rendered.contains("cargo build ||"), "{rendered}"); + assert!(rendered.contains("echo fallback"), "{rendered}"); + } + /// 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 diff --git a/docs/V0_9_0_EXECUTION_MAP.md b/docs/V0_9_0_EXECUTION_MAP.md index d299d59e..6a8a1830 100644 --- a/docs/V0_9_0_EXECUTION_MAP.md +++ b/docs/V0_9_0_EXECUTION_MAP.md @@ -96,7 +96,7 @@ v0.9 branch so the remaining Windows/manual checks are explicit. | #2239 i18n Phase 1-4b | Conflicting | Defer until localization lane. | | #2242 typed persistent tool permission rules | Conflicting | Compare with #2721 stabilization and permissions model. | | #2256 workspace crate consolidation | Conflicting | Do not merge during v0.9 stabilization. | -| #2269 approval details and shell previews | Conflicting | Review for small UI harvest only. | +| #2269 approval details and shell previews | Conflicting / locally harvested | Narrow UI slice landed manually: approval cards now show prominent command/dir/file/path/target rows, preserve #2381 intent summaries, classify live shell companion tools as shell, split common shell chains, and show compact simple `printf > file` previews. Do not merge the broader diff-preview/pager rewrite. Close/comment after branch is public, crediting @tdccccc for #1991/#2269. | | #2318 message_submit hook transform | Draft/conflicting | Defer; hook behavior must match lifecycle policy. | | #2382 v0.8.48 release harvest | Draft/conflicting | Candidate to close as obsolete after confirming no unharvested commits. | | #2476 fork migration parent links | Conflicting / already harvested | Patch-equivalent work is already present on `origin/main` and this branch as `b76a11b99` plus follow-up `18550339a`. Close/comment original after the integration branch is public, crediting @cyq1017; close issue #2082 only after confirming the remaining `message_type` wording is obsolete. |