From aa0e54a9e32c4f8b4c3dadf7df877e1f99f0be27 Mon Sep 17 00:00:00 2001 From: z27014 Date: Sat, 5 Sep 2026 23:39:56 +0800 Subject: [PATCH 1/3] fix: align shell guidance with execution Signed-off-by: z27014 --- crates/tui/src/tools/shell.rs | 5 +- crates/tui/src/tools/shell/guidance.rs | 150 +++++++++++++++++++++++++ crates/tui/src/tools/shell/tests.rs | 52 +++++++++ 3 files changed, 205 insertions(+), 2 deletions(-) create mode 100644 crates/tui/src/tools/shell/guidance.rs diff --git a/crates/tui/src/tools/shell.rs b/crates/tui/src/tools/shell.rs index 87ea13543c..09e6b49c64 100644 --- a/crates/tui/src/tools/shell.rs +++ b/crates/tui/src/tools/shell.rs @@ -44,6 +44,7 @@ use windows::core::PCWSTR; #[cfg(not(target_env = "ohos"))] use portable_pty::{CommandBuilder, PtySize, native_pty_system}; +mod guidance; mod output; use super::shell_output::{summarize_output, truncate_with_meta}; @@ -4541,7 +4542,7 @@ impl ToolSpec for BashTool { if self.read_only { "Inspect the workspace with the bounded read-only command subset. Commands run directly as argv, never through a shell; only action=run plus command, cwd, and timeout_ms are accepted." } else { - "Execute a shell command in the workspace. Action \"run\" (default) executes a command; \"wait\" blocks for a background task until completion or timeout; \"interact\" sends stdin to a background task; \"cancel\" kills a background task. Pass wait=false for a nonblocking task snapshot. Foreground mode is for bounded commands; use background=true for work expected to take >5 seconds. Commands run via the user's login shell ($SHELL); when that shell is zsh, a bare word starting with `=` undergoes `=command` PATH expansion (e.g. `echo ===` fails) — quote such arguments, e.g. `echo '==='`." + guidance::description() } } @@ -4559,7 +4560,7 @@ impl ToolSpec for BashTool { }, "command": { "type": "string", - "description": "The shell command to execute (action=run)" + "description": guidance::runtime_command_guidance() }, "timeout_ms": { "type": "integer", diff --git a/crates/tui/src/tools/shell/guidance.rs b/crates/tui/src/tools/shell/guidance.rs new file mode 100644 index 0000000000..519e6344c7 --- /dev/null +++ b/crates/tui/src/tools/shell/guidance.rs @@ -0,0 +1,150 @@ +//! Model-facing command syntax follows the same dispatcher as execution. + +use crate::shell_dispatcher::{ShellKind, global_dispatcher}; +use std::sync::OnceLock; + +const POWERSHELL_GUIDANCE: &str = "Use PowerShell syntax. Bash is a legacy tool name, not a Bash interpreter. \ + For JSON use Invoke-RestMethod; for text use Invoke-WebRequest -UseBasicParsing \ + on Windows PowerShell to avoid dependency on the Internet Explorer engine. \ + Do not assume head, sed, awk, or other Unix utilities are installed. \ + Use PowerShell cmdlets or verified available programs; parse JSON and select needed fields \ + instead of appending head. Bash heredocs are not PowerShell syntax. Use PowerShell \ + 5.1-compatible syntax (no && or ||) unless the detected executable is pwsh. Example: \ + $text = 'sample'; $text.Substring(0, [Math]::Min(3, $text.Length))."; + +const BASH_GUIDANCE: &str = "Use Bash syntax: pipelines, redirections, $(command), \ + and && / || are supported. Quote paths and variable expansions, such as \"$path\"; \ + use single quotes for literal text. For literal multiline input, use a quoted heredoc \ + delimiter (<<'EOF') with its closing delimiter on a separate line. Use only installed \ + programs; do not assume GNU-specific flags on macOS/BSD. Example: printf '%s\\n' 'sample'."; + +const SH_GUIDANCE: &str = "Use POSIX sh syntax: pipelines, redirections, $(command), \ + and && / || are supported. Quote paths and variable expansions, such as \"$path\"; \ + use single quotes for literal text. Do not use Bash-only arrays, [[ ... ]], \ + process substitution, or here-strings. Use only installed programs and portable \ + utility options. Example: printf '%s\\n' 'sample'."; + +const ZSH_GUIDANCE: &str = "Use zsh syntax. Quote paths, literal wildcard patterns, \ + and variable expansions; unmatched unquoted globs can fail before a command runs. \ + A bare word starting with = undergoes =command PATH expansion (e.g. echo === fails); \ + quote such arguments, e.g. echo '==='. Do not assume Bash array indexing or word \ + splitting rules. Use only installed programs; do not assume GNU-specific flags on macOS/BSD."; + +const CMD_GUIDANCE: &str = "Use cmd.exe syntax: %NAME% expands environment variables; use double quotes \ + around paths containing spaces (single quotes are not quoting delimiters). \ + Use cmd built-ins or installed programs, not Bash or PowerShell syntax. \ + Do not assume Unix utilities are installed. Example: echo sample"; + +const FISH_GUIDANCE: &str = "Use fish syntax: set NAME value for variables, \ + and begin ... end for blocks. Bash assignment NAME=value and \ + heredocs are not portable fish syntax. Quote paths and use only \ + installed programs. Example: printf '%s\\n' 'sample'."; + +const FALLBACK_GUIDANCE: &str = "Use the detected shell's syntax and only installed programs; \ + do not infer Bash syntax from the legacy tool name."; + +pub(super) fn command_guidance(kind: &ShellKind) -> String { + let syntax = match kind { + // Match execution's PowerShell-family detection, including custom paths. + _ if kind.is_powershell() => POWERSHELL_GUIDANCE, + ShellKind::Cmd => CMD_GUIDANCE, + ShellKind::Sh => SH_GUIDANCE, + ShellKind::Bash => BASH_GUIDANCE, + ShellKind::Custom { binary, .. } => { + match std::path::Path::new(binary) + .file_stem() + .and_then(|name| name.to_str()) + .map(str::to_ascii_lowercase) + .as_deref() + { + Some("bash") => BASH_GUIDANCE, + Some("sh" | "dash" | "ash") => SH_GUIDANCE, + Some("zsh") => ZSH_GUIDANCE, + Some("fish") => FISH_GUIDANCE, + _ => FALLBACK_GUIDANCE, + } + } + _ => FALLBACK_GUIDANCE, + }; + format!( + "The command to execute (action=run). Actual execution shell: `{}`. {syntax}", + kind.binary() + ) +} + +pub(super) fn runtime_command_guidance() -> &'static str { + static GUIDANCE: OnceLock = OnceLock::new(); + GUIDANCE.get_or_init(|| command_guidance(global_dispatcher().kind())) +} + +pub(super) fn description() -> &'static str { + static DESCRIPTION: OnceLock = OnceLock::new(); + DESCRIPTION.get_or_init(|| { + format!( + "{} Execute in the workspace. Action \"run\" (default) executes a command; \ + \"wait\" blocks for a background task until completion or timeout; \"interact\" sends stdin to a background task; \ + \"cancel\" kills a background task. Pass wait=false for a nonblocking task snapshot. Foreground mode is for bounded commands; \ + use background=true for work expected to take >5 seconds.", + runtime_command_guidance() + ) + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn shell_guidance_preserves_unix_shell_contracts() { + for (binary, expected) in [ + ("/bin/bash", BASH_GUIDANCE), + ("bash", BASH_GUIDANCE), + ("/usr/local/bin/bash", BASH_GUIDANCE), + ("/bin/sh", SH_GUIDANCE), + ("/bin/dash", SH_GUIDANCE), + ("/bin/ash", SH_GUIDANCE), + ("/bin/zsh", ZSH_GUIDANCE), + ] { + let text = command_guidance(&ShellKind::Custom { + binary: binary.into(), + flag: "-lc".into(), + }); + assert!(text.contains(expected), "missing guidance for {binary}"); + assert!(!text.contains("Use PowerShell syntax")); + } + assert!(command_guidance(&ShellKind::Bash).contains(BASH_GUIDANCE)); + assert!(command_guidance(&ShellKind::Sh).contains(SH_GUIDANCE)); + } + + #[test] + fn shell_guidance_matches_each_interpreter() { + for kind in [ + ShellKind::Pwsh, + ShellKind::WindowsPowerShell, + ShellKind::Cmd, + ShellKind::Sh, + ShellKind::Bash, + ShellKind::Custom { + binary: "/bin/zsh".into(), + flag: "-lc".into(), + }, + ShellKind::Custom { + binary: "/opt/pwsh".into(), + flag: "-c".into(), + }, + ShellKind::Custom { + binary: "/bin/fish".into(), + flag: "-c".into(), + }, + ] { + let text = command_guidance(&kind); + assert!(text.contains(kind.binary())); + assert_eq!(text.contains("Use PowerShell syntax"), kind.is_powershell()); + assert_eq!( + text.contains("=command PATH expansion"), + kind.binary() == "/bin/zsh" + ); + assert!(!text.contains("user's login shell")); + } + } +} diff --git a/crates/tui/src/tools/shell/tests.rs b/crates/tui/src/tools/shell/tests.rs index 4c78aeee76..b722673725 100644 --- a/crates/tui/src/tools/shell/tests.rs +++ b/crates/tui/src/tools/shell/tests.rs @@ -21,6 +21,58 @@ fn env_lock() -> &'static Mutex<()> { const BACKGROUND_COMPLETION_WAIT_MS: u64 = 30_000; +#[test] +fn shell_catalog_guidance_matches_execution() { + let tool = BashTool::new("Bash"); + let schema = tool.input_schema(); + let command = schema["properties"]["command"]["description"] + .as_str() + .unwrap(); + let dispatcher = crate::shell_dispatcher::global_dispatcher(); + assert!(command.contains(dispatcher.kind().binary())); + assert!(tool.description().contains(command)); + assert_eq!(tool.name(), "Bash"); + assert!(tool.model_visible()); + assert!(tool.description().contains("background=true")); + let readonly = BashTool::read_only("Bash"); + assert!(readonly.description().contains("never through a shell")); + assert!( + !readonly + .input_schema() + .to_string() + .contains("Actual execution shell") + ); + let alias = BashTool::alias("exec_shell", "run"); + assert_eq!(alias.description(), tool.description()); + let workspace = tempdir().unwrap(); + let mut registry = crate::tools::ToolRegistry::new(ToolContext::new(workspace.path())); + registry.register(std::sync::Arc::new(BashTool::new("Bash"))); + let catalog = registry.to_api_tools(); + assert_eq!(catalog.len(), 1); + assert_eq!(catalog[0].description, tool.description()); + assert_eq!( + catalog[0].input_schema["properties"]["command"]["description"], + command + ); +} + +#[test] +#[ignore = "Exports model-visible shell fixtures for opt-in live model evaluation"] +fn export_shell_guidance_eval_fixture() { + let path = std::env::var_os("SHELL_GUIDANCE_FIXTURE").expect("SHELL_GUIDANCE_FIXTURE"); + let tool = BashTool::new("Bash"); + let mut schema = tool.input_schema(); + crate::tools::schema_sanitize::sanitize(&mut schema); + crate::tools::schema_canonicalize::canonicalize_schema(&mut schema); + let fixture = json!({ + "name": tool.name(), + "description": tool.description(), + "input_schema": schema, + "shell": crate::shell_dispatcher::global_dispatcher().kind().binary(), + }); + std::fs::write(path, serde_json::to_vec_pretty(&fixture).unwrap()).unwrap(); +} + #[test] fn lowercase_bash_schema_is_small_contract() { let schema = LowercaseBashTool.input_schema(); From 47c719b8a381bc0aedf7a8f35f511f79e3ee2438 Mon Sep 17 00:00:00 2001 From: z27014 Date: Sat, 5 Sep 2026 23:45:39 +0800 Subject: [PATCH 2/3] fix: guide the visible lowercase shell tool Signed-off-by: z27014 --- crates/tui/src/tools/shell.rs | 4 ++-- crates/tui/src/tools/shell/guidance.rs | 12 +++++++++- crates/tui/src/tools/shell/tests.rs | 32 +++++++++++++++++--------- 3 files changed, 34 insertions(+), 14 deletions(-) diff --git a/crates/tui/src/tools/shell.rs b/crates/tui/src/tools/shell.rs index 09e6b49c64..2921bf692e 100644 --- a/crates/tui/src/tools/shell.rs +++ b/crates/tui/src/tools/shell.rs @@ -4361,14 +4361,14 @@ impl ToolSpec for LowercaseBashTool { } fn description(&self) -> &'static str { - "Execute a shell command in the workspace and return stdout and stderr. Output keeps the last 2000 lines or 50KB. An optional timeout is expressed in seconds; when omitted the command is killed after 120 seconds, so pass an explicit timeout for work expected to take longer. In Ask, after a sandbox denial, retry the exact command once with sandbox_permissions (the narrowest wider mode that suffices) and a one-sentence justification; the approval prompt asks the user." + guidance::foreground_description() } fn input_schema(&self) -> serde_json::Value { json!({ "type": "object", "properties": { - "command": { "type": "string", "description": "Bash command to execute." }, + "command": { "type": "string", "description": guidance::runtime_command_guidance() }, "timeout": { "type": "number", "description": "Optional timeout in seconds; when omitted the command is killed after 120 seconds." }, "sandbox_permissions": { "type": "string", diff --git a/crates/tui/src/tools/shell/guidance.rs b/crates/tui/src/tools/shell/guidance.rs index 519e6344c7..5655a78609 100644 --- a/crates/tui/src/tools/shell/guidance.rs +++ b/crates/tui/src/tools/shell/guidance.rs @@ -67,7 +67,7 @@ pub(super) fn command_guidance(kind: &ShellKind) -> String { _ => FALLBACK_GUIDANCE, }; format!( - "The command to execute (action=run). Actual execution shell: `{}`. {syntax}", + "The command to execute. Actual execution shell: `{}`. {syntax}", kind.binary() ) } @@ -90,6 +90,16 @@ pub(super) fn description() -> &'static str { }) } +pub(super) fn foreground_description() -> &'static str { + static DESCRIPTION: OnceLock = OnceLock::new(); + DESCRIPTION.get_or_init(|| { + format!( + "{} Execute a shell command in the workspace and return stdout and stderr. Output keeps the last 2000 lines or 50KB. An optional timeout is expressed in seconds; when omitted the command is killed after 120 seconds, so pass an explicit timeout for work expected to take longer. In Ask, after a sandbox denial, retry the exact command once with sandbox_permissions (the narrowest wider mode that suffices) and a one-sentence justification; the approval prompt asks the user.", + runtime_command_guidance() + ) + }) +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/tui/src/tools/shell/tests.rs b/crates/tui/src/tools/shell/tests.rs index b722673725..ba8bcfb4a0 100644 --- a/crates/tui/src/tools/shell/tests.rs +++ b/crates/tui/src/tools/shell/tests.rs @@ -23,7 +23,7 @@ const BACKGROUND_COMPLETION_WAIT_MS: u64 = 30_000; #[test] fn shell_catalog_guidance_matches_execution() { - let tool = BashTool::new("Bash"); + let tool = LowercaseBashTool; let schema = tool.input_schema(); let command = schema["properties"]["command"]["description"] .as_str() @@ -31,9 +31,15 @@ fn shell_catalog_guidance_matches_execution() { let dispatcher = crate::shell_dispatcher::global_dispatcher(); assert!(command.contains(dispatcher.kind().binary())); assert!(tool.description().contains(command)); - assert_eq!(tool.name(), "Bash"); + assert_eq!(tool.name(), "bash"); assert!(tool.model_visible()); - assert!(tool.description().contains("background=true")); + assert!(!command.contains("action=run")); + assert!(!tool.description().contains("background=true")); + let legacy = BashTool::new("Bash"); + assert!(!legacy.model_visible()); + assert!(legacy.description().contains(command)); + assert!(legacy.description().contains("background=true")); + assert!(legacy.description().contains("wait=false")); let readonly = BashTool::read_only("Bash"); assert!(readonly.description().contains("never through a shell")); assert!( @@ -43,12 +49,14 @@ fn shell_catalog_guidance_matches_execution() { .contains("Actual execution shell") ); let alias = BashTool::alias("exec_shell", "run"); - assert_eq!(alias.description(), tool.description()); + assert_eq!(alias.description(), legacy.description()); let workspace = tempdir().unwrap(); let mut registry = crate::tools::ToolRegistry::new(ToolContext::new(workspace.path())); registry.register(std::sync::Arc::new(BashTool::new("Bash"))); + registry.register(std::sync::Arc::new(LowercaseBashTool)); let catalog = registry.to_api_tools(); assert_eq!(catalog.len(), 1); + assert_eq!(catalog[0].name, "bash"); assert_eq!(catalog[0].description, tool.description()); assert_eq!( catalog[0].input_schema["properties"]["command"]["description"], @@ -60,14 +68,16 @@ fn shell_catalog_guidance_matches_execution() { #[ignore = "Exports model-visible shell fixtures for opt-in live model evaluation"] fn export_shell_guidance_eval_fixture() { let path = std::env::var_os("SHELL_GUIDANCE_FIXTURE").expect("SHELL_GUIDANCE_FIXTURE"); - let tool = BashTool::new("Bash"); - let mut schema = tool.input_schema(); - crate::tools::schema_sanitize::sanitize(&mut schema); - crate::tools::schema_canonicalize::canonicalize_schema(&mut schema); + let workspace = tempdir().unwrap(); + let mut registry = crate::tools::ToolRegistry::new(ToolContext::new(workspace.path())); + registry.register(std::sync::Arc::new(BashTool::new("Bash"))); + registry.register(std::sync::Arc::new(LowercaseBashTool)); + let catalog = registry.to_api_tools(); + let tool = catalog.iter().find(|tool| tool.name == "bash").unwrap(); let fixture = json!({ - "name": tool.name(), - "description": tool.description(), - "input_schema": schema, + "name": tool.name, + "description": tool.description, + "input_schema": tool.input_schema, "shell": crate::shell_dispatcher::global_dispatcher().kind().binary(), }); std::fs::write(path, serde_json::to_vec_pretty(&fixture).unwrap()).unwrap(); From 985f38b60924a0e40b17547b7e6e2705854b0ca0 Mon Sep 17 00:00:00 2001 From: z27014 Date: Sun, 6 Sep 2026 09:25:57 +0800 Subject: [PATCH 3/3] test: preserve shell sandbox retry guidance Signed-off-by: z27014 --- crates/tui/src/tools/shell/tests.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/tui/src/tools/shell/tests.rs b/crates/tui/src/tools/shell/tests.rs index ba8bcfb4a0..588fc93fd0 100644 --- a/crates/tui/src/tools/shell/tests.rs +++ b/crates/tui/src/tools/shell/tests.rs @@ -35,6 +35,12 @@ fn shell_catalog_guidance_matches_execution() { assert!(tool.model_visible()); assert!(!command.contains("action=run")); assert!(!tool.description().contains("background=true")); + assert!( + tool.description().contains( + "In Ask, after a sandbox denial, retry the exact command once with sandbox_permissions (the narrowest wider mode that suffices) and a one-sentence justification; the approval prompt asks the user." + ), + "foreground guidance must preserve the sandbox retry and approval contract" + ); let legacy = BashTool::new("Bash"); assert!(!legacy.model_visible()); assert!(legacy.description().contains(command));