diff --git a/SHELL_AWARENESS_README.md b/SHELL_AWARENESS_README.md new file mode 100644 index 000000000..f88e418d0 --- /dev/null +++ b/SHELL_AWARENESS_README.md @@ -0,0 +1,209 @@ +# Shell Awareness Feature for Amazon Q CLI + +## Overview + +This feature adds shell awareness to Amazon Q CLI, allowing it to detect and display the correct shell-specific tool name (e.g., `execute_zsh` instead of `execute_bash`) when running commands. + +## The Problem + +The original Q CLI always showed `execute_bash` in the UI, regardless of what shell the user was actually running. This was misleading for users running zsh, fish, or other shells, as it suggested the tool wasn't aware of their shell environment. + +### Root Cause Analysis + +The issue was caused by **three separate shell detection systems** that weren't coordinated: + +1. **Tool Creation System** (`tool_manager.rs`): Created shell-specific tools but used basic detection +2. **Execute Module** (`execute/mod.rs`): Had its own simple shell detection for permissions/settings +3. **UI Display System** (`tools/mod.rs`): Had hardcoded shell detection for display names + +Each system used different detection logic: +- Simple detection: Just checked `$SHELL` environment variable and split on `/` +- The problem: `$SHELL` shows the **login shell** (often bash), not the **current shell** (e.g., zsh) + +### Specific Issues Found + +1. **Environment Variable Mismatch**: Even in zsh sessions, `$SHELL` often shows `/bin/bash` because that's the login shell +2. **Missing Shell-Specific Variables**: `$ZSH_VERSION` wasn't available to child processes in some configurations +3. **Inconsistent Detection**: Three different detection methods led to mismatched results +4. **Hardcoded Tool References**: Some code still had hardcoded `execute_bash` references + +## The Solution + +### Unified Shell Detection + +We implemented a **unified shell detection system** that all three components use: + +```rust +pub fn detect_shell_for_execute() -> String { + // Method 1: Check shell-specific environment variables + if env::var("ZSH_VERSION").is_ok() { + return "zsh".to_string(); + } + if env::var("BASH_VERSION").is_ok() { + return "bash".to_string(); + } + + // Method 2: Active detection - test if zsh is available and working + if let Ok(output) = Command::new("zsh").args(&["-c", "echo $ZSH_VERSION"]).output() { + if output.status.success() { + if let Ok(version) = String::from_utf8(output.stdout) { + let version = version.trim(); + if !version.is_empty() { + return "zsh".to_string(); + } + } + } + } + + // Method 3: Fallback to SHELL environment variable + if let Ok(shell_path) = env::var("SHELL") { + if let Some(shell_name) = shell_path.split('/').last() { + match shell_name { + "zsh" => return "zsh".to_string(), + "bash" => return "bash".to_string(), + // ... other shells + } + } + } + + // Default fallback + "bash".to_string() +} +``` + +### Key Improvements + +1. **Active Detection**: Tests if zsh is actually available by running `zsh -c "echo $ZSH_VERSION"` +2. **Unified Function**: All three systems now call the same detection function +3. **Consistent Results**: Tool creation, execution, and display all show the same shell +4. **Removed Hardcoded References**: Eliminated hardcoded `execute_bash` references + +### Files Modified + +- `crates/chat-cli/src/cli/chat/tool_manager.rs`: Unified tool creation +- `crates/chat-cli/src/cli/chat/tools/execute/mod.rs`: Updated execute module detection +- `crates/chat-cli/src/cli/chat/tools/mod.rs`: Fixed UI display detection +- `crates/chat-cli/src/cli/chat/tools/mod.rs`: Removed hardcoded `execute_bash` from tool list + +## Testing the Feature + +### Prerequisites + +Since this feature isn't in the released version of Q CLI, you'll need to build it from source: + +1. **Get the Code**: + ``` + # Clone the repository (or use existing clone) + git clone https://github.com/aws/amazon-q-developer-cli.git + cd amazon-q-developer-cli + git checkout shell-awareness-feature + ``` + +2. **Install Rust** (if not already installed): + ``` + curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh + rustup default stable + ``` + +### Building and Testing + +1. **Build the Project**: + ``` + cargo build --release + ``` + +2. **Test the Feature**: + ``` + # Run the built version directly (don't install it to avoid conflicts) + ./target/release/chat_cli + ``` + +### Expected Behavior + +#### Before the Fix +``` +🛠️ Using tool: execute_bash (trusted) +``` +(Even when running in zsh) + +#### After the Fix +``` +🛠️ Using tool: execute_zsh (trusted) +``` +(When running in zsh) + +### Test Cases + +1. **In zsh**: Should show `execute_zsh` +2. **In bash**: Should show `execute_bash` +3. **In fish**: Should show `execute_fish` (if fish is detected) +4. **Unknown shells**: Should fall back to `execute_bash` + +### Debug Output + +The feature includes debug output to help verify detection: + +``` +DEBUG: Unix shell detection starting... +DEBUG: Detected shell: zsh +DEBUG: Creating tool: execute_zsh +DEBUG: Execute module detected shell: zsh +DEBUG: Execute module using tool_name: execute_zsh +``` + +## Limitations and Future Work + +### Current Limitations + +1. **Fish Detection**: May not work perfectly due to fish's unique environment variable handling +2. **Complex Shell Setups**: May not detect shells in complex nested or containerized environments +3. **Performance**: Active detection adds a small startup cost (running `zsh -c` command) + +### Shell Support Status + +- ✅ **zsh**: Fully supported with active detection +- ✅ **bash**: Fully supported +- ⚠️ **fish**: Basic support (may not detect reliably) +- ⚠️ **Other shells**: Basic support via `$SHELL` variable parsing + +### Future Improvements + +1. **Better Fish Detection**: Implement fish-specific detection methods +2. **Caching**: Cache shell detection results to improve performance +3. **Configuration Override**: Allow users to manually specify their shell +4. **Extended Shell Support**: Add detection for more shells (ksh, tcsh, etc.) + +## Contributing + +If you encounter issues with shell detection or want to improve support for additional shells, please: + +1. Fork the repository +2. Create a feature branch +3. Add debug output to understand the detection behavior +4. Test with your specific shell configuration +5. Submit a pull request with improvements + +## Troubleshooting + +### Shell Not Detected Correctly + +1. **Check Environment Variables**: + ``` + echo "SHELL: $SHELL" + echo "ZSH_VERSION: $ZSH_VERSION" + echo "BASH_VERSION: $BASH_VERSION" + ``` + +2. **Test Active Detection**: + ``` + zsh -c "echo $ZSH_VERSION" # Should return version if zsh works + ``` + +3. **Check Debug Output**: Look for the debug messages during Q CLI startup + +### Performance Issues + +If shell detection is slow: +1. The active detection method may be taking time +2. Consider using a simpler detection method for your use case +3. File an issue with your shell configuration details diff --git a/crates/chat-cli/src/cli/chat/tool_manager.rs b/crates/chat-cli/src/cli/chat/tool_manager.rs index 95739a106..d0154e1ac 100644 --- a/crates/chat-cli/src/cli/chat/tool_manager.rs +++ b/crates/chat-cli/src/cli/chat/tool_manager.rs @@ -103,6 +103,110 @@ const NAMESPACE_DELIMITER: &str = "___"; const VALID_TOOL_NAME: &str = "^[a-zA-Z][a-zA-Z0-9_]*$"; const SPINNER_CHARS: [char; 10] = ['⠋', '⠙', '⠹', '⠸', '⠼', '⠴', '⠦', '⠧', '⠇', '⠏']; +/// Detects the actual shell being used on Windows systems +#[cfg(windows)] +fn detect_shell_windows() -> String { + use std::env; + + // Check for PowerShell Core (pwsh) first + if env::var("PSModulePath").is_ok() { + if let Ok(ps_version) = env::var("PSVersionTable") { + if ps_version.contains("Core") { + return "pwsh".to_string(); + } + } + // Check if we're in PowerShell (any version) + if env::var("PSMODULEPATH").is_ok() || env::var("PSExecutionPolicyPreference").is_ok() { + return "powershell".to_string(); + } + } + + // Check for Command Prompt indicators + if env::var("PROMPT").is_ok() || env::var("COMSPEC").is_ok() { + return "cmd".to_string(); + } + + // Default to cmd for Windows + "cmd".to_string() +} + +/// Detects the actual shell being used on Unix-like systems +#[cfg(not(windows))] +/// Simple shell detection for Unix-like systems +#[cfg(not(windows))] +pub fn detect_shell_for_execute() -> String { + detect_shell_unix_simple() +} + +/// Simple shell detection for Unix-like systems +#[cfg(not(windows))] +fn detect_shell_unix_simple() -> String { + use std::env; + use std::process::Command; + + eprintln!("DEBUG: SHELL={:?}", env::var("SHELL")); + eprintln!("DEBUG: ZSH_VERSION={:?}", env::var("ZSH_VERSION")); + eprintln!("DEBUG: BASH_VERSION={:?}", env::var("BASH_VERSION")); + + // Method 1: Check for shell-specific environment variables (most reliable) + if env::var("ZSH_VERSION").is_ok() { + eprintln!("DEBUG: Found ZSH_VERSION, returning zsh"); + return "zsh".to_string(); + } + if env::var("BASH_VERSION").is_ok() { + eprintln!("DEBUG: Found BASH_VERSION, returning bash"); + return "bash".to_string(); + } + + // Method 2: Try to detect by checking what shell can run zsh-specific commands + if let Ok(output) = Command::new("zsh").args(&["-c", "echo $ZSH_VERSION"]).output() { + if output.status.success() { + if let Ok(version) = String::from_utf8(output.stdout) { + let version = version.trim(); + if !version.is_empty() { + eprintln!("DEBUG: zsh command returned version: {}, using zsh", version); + return "zsh".to_string(); + } + } + } + } + + // Method 3: Check the SHELL environment variable (login shell fallback) + if let Ok(shell_path) = env::var("SHELL") { + eprintln!("DEBUG: SHELL path: {}", shell_path); + if let Some(shell_name) = shell_path.split('/').last() { + eprintln!("DEBUG: Shell name from path: {}", shell_name); + match shell_name { + "bash" => { + eprintln!("DEBUG: Matched bash from SHELL path"); + return "bash".to_string(); + }, + "zsh" => { + eprintln!("DEBUG: Matched zsh from SHELL path"); + return "zsh".to_string(); + }, + "fish" => return "fish".to_string(), + "sh" => return "sh".to_string(), + "dash" => return "dash".to_string(), + "ksh" => return "ksh".to_string(), + "tcsh" => return "tcsh".to_string(), + "csh" => return "csh".to_string(), + _ => { + // For unknown shells, return the name as-is if it's alphanumeric + if shell_name.chars().all(|c| c.is_alphanumeric()) { + eprintln!("DEBUG: Unknown shell, returning as-is: {}", shell_name); + return shell_name.to_string(); + } + } + } + } + } + + eprintln!("DEBUG: No shell detected, defaulting to bash"); + // Default fallback + "bash".to_string() +} + pub fn workspace_mcp_config_path(os: &Os) -> eyre::Result { Ok(os.env.current_dir()?.join(".amazonq").join("mcp.json")) } @@ -937,6 +1041,121 @@ impl ToolManager { "required": ["command"]})), tool_origin: ToolOrigin::Native, }); +<<<<<<< Updated upstream +======= + + // Create backward compatibility alias + if tool_name != "execute_cmd" { + tool_specs.insert("execute_cmd".to_string(), ToolSpec { + name: "execute_cmd".to_string(), + description: "Execute the specified Windows command.".to_string(), + input_schema: InputSchema(json!({ + "type": "object", + "properties": { + "command": { + "type": "string", + "description": "Windows command to execute" + }, + "summary": { + "type": "string", + "description": "A brief explanation of what the command does" + } + }, + "required": ["command"]})), + tool_origin: ToolOrigin::Native, + }); + } + } + + #[cfg(not(windows))] + { + use serde_json::json; + + use crate::cli::chat::tools::InputSchema; + + eprintln!("DEBUG: Unix shell detection starting..."); + + // EXPERIMENT: Always remove execute_bash completely + let removed = tool_specs.remove("execute_bash"); + eprintln!("DEBUG: Removed execute_bash: {:?}", removed.is_some()); + + // Detect the actual shell on Unix-like systems + let shell_name = detect_shell_unix_simple(); + eprintln!("DEBUG: Detected shell: {}", shell_name); + let tool_name = format!("execute_{}", shell_name); + eprintln!("DEBUG: Creating tool: {}", tool_name); + + // Debug: Show what execute tools are in tool_specs + let execute_tools: Vec<_> = tool_specs.keys() + .filter(|k| k.starts_with("execute_")) + .collect(); + eprintln!("DEBUG: Execute tools before adding new one: {:?}", execute_tools); + let description = format!("Execute the specified {} command.", shell_name); + + // Only create the shell-specific tool (no bash fallback) + tool_specs.insert(tool_name.clone(), ToolSpec { + name: tool_name.clone(), + description, + input_schema: InputSchema(json!({ + "type": "object", + "properties": { + "command": { + "type": "string", + "description": format!("{} command to execute", shell_name) + }, + "summary": { + "type": "string", + "description": "A brief explanation of what the command does" + } + }, + "required": ["command"]})), + tool_origin: ToolOrigin::Native, + }); + + // Debug: Show what execute tools are in tool_specs after adding new one + let execute_tools_after: Vec<_> = tool_specs.keys() + .filter(|k| k.starts_with("execute_")) + .collect(); + eprintln!("DEBUG: Execute tools after adding new one: {:?}", execute_tools_after); + + // Debug: Show ALL tools available to model + let all_tools: Vec<_> = tool_specs.keys().collect(); + eprintln!("DEBUG: ALL tools available: {:?}", all_tools); + + // Debug: Show what execute tools are in tool_specs + let execute_tools: Vec<_> = tool_specs.keys() + .filter(|k| k.starts_with("execute_")) + .collect(); + eprintln!("DEBUG: Execute tools before adding new one: {:?}", execute_tools); + let description = format!("Execute the specified {} command.", shell_name); + + // Only create the shell-specific tool (no bash fallback) + tool_specs.insert(tool_name.clone(), ToolSpec { + name: tool_name.clone(), + description, + input_schema: InputSchema(json!({ + "type": "object", + "properties": { + "command": { + "type": "string", + "description": format!("{} command to execute", shell_name) + }, + "summary": { + "type": "string", + "description": "A brief explanation of what the command does" + } + }, + "required": ["command"]})), + tool_origin: ToolOrigin::Native, + }); + + // Debug: Show what execute tools are in tool_specs after adding new one + let execute_tools_after: Vec<_> = tool_specs.keys() + .filter(|k| k.starts_with("execute_")) + .collect(); + eprintln!("DEBUG: Execute tools after adding new one: {:?}", execute_tools_after); + + // EXPERIMENT: No backward compatibility - force shell-specific tool usage } tool_specs diff --git a/crates/chat-cli/src/cli/chat/tools/execute/mod.rs b/crates/chat-cli/src/cli/chat/tools/execute/mod.rs index 7ab8612b7..378034215 100644 --- a/crates/chat-cli/src/cli/chat/tools/execute/mod.rs +++ b/crates/chat-cli/src/cli/chat/tools/execute/mod.rs @@ -192,9 +192,21 @@ impl ExecuteCommand { } let Self { command, .. } = self; - let tool_name = if cfg!(windows) { "execute_cmd" } else { "execute_bash" }; - let is_in_allowlist = agent.allowed_tools.contains("execute_bash"); - match agent.tools_settings.get(tool_name) { + + // Detect the actual shell dynamically + let detected_shell = if cfg!(windows) { + "cmd".to_string() + } else { + // Use the same shell detection logic as tool creation + let shell = crate::cli::chat::tool_manager::detect_shell_for_execute(); + eprintln!("DEBUG: Execute module detected shell: {}", shell); + shell + }; + + let tool_name = format!("execute_{}", detected_shell); + eprintln!("DEBUG: Execute module using tool_name: {}", tool_name); + let is_in_allowlist = agent.allowed_tools.contains(&tool_name) || agent.allowed_tools.contains("execute_bash"); + match agent.tools_settings.get(tool_name.as_str()).or_else(|| agent.tools_settings.get("execute_bash")) { Some(settings) if is_in_allowlist => { let Settings { allowed_commands, @@ -203,7 +215,7 @@ impl ExecuteCommand { } = match serde_json::from_value::(settings.clone()) { Ok(settings) => settings, Err(e) => { - error!("Failed to deserialize tool settings for execute_bash: {:?}", e); + error!("Failed to deserialize tool settings for {}: {:?}", tool_name, e); return PermissionEvalResult::Ask; }, }; diff --git a/crates/chat-cli/src/cli/chat/tools/mod.rs b/crates/chat-cli/src/cli/chat/tools/mod.rs index 8788a7c7d..107612595 100644 --- a/crates/chat-cli/src/cli/chat/tools/mod.rs +++ b/crates/chat-cli/src/cli/chat/tools/mod.rs @@ -46,13 +46,12 @@ use crate::cli::agent::{ use crate::os::Os; pub const DEFAULT_APPROVE: [&str; 1] = ["fs_read"]; -pub const NATIVE_TOOLS: [&str; 7] = [ +pub const NATIVE_TOOLS: [&str; 6] = [ "fs_read", "fs_write", #[cfg(windows)] "execute_cmd", - #[cfg(not(windows))] - "execute_bash", + // Note: execute_bash removed - shell-specific tools are now created dynamically "use_aws", "gh_issue", "knowledge", @@ -82,7 +81,18 @@ impl Tool { #[cfg(windows)] Tool::ExecuteCommand(_) => "execute_cmd", #[cfg(not(windows))] - Tool::ExecuteCommand(_) => "execute_bash", + Tool::ExecuteCommand(_) => { + // Use the same unified shell detection as tool creation and execution + #[cfg(not(windows))] + { + let detected_shell = crate::cli::chat::tool_manager::detect_shell_for_execute(); + return format!("execute_{}", detected_shell); + } + #[cfg(windows)] + { + return "execute_cmd".to_string(); + } + }, Tool::UseAws(_) => "use_aws", Tool::Custom(custom_tool) => &custom_tool.name, Tool::GhIssue(_) => "gh_issue", diff --git a/crates/chat-cli/src/cli/chat/tools/tool_index.json b/crates/chat-cli/src/cli/chat/tools/tool_index.json index 067e5df1e..af359dcc0 100644 --- a/crates/chat-cli/src/cli/chat/tools/tool_index.json +++ b/crates/chat-cli/src/cli/chat/tools/tool_index.json @@ -8,26 +8,6 @@ "required": [] } }, - "execute_bash": { - "name": "execute_bash", - "description": "Execute the specified bash command.", - "input_schema": { - "type": "object", - "properties": { - "command": { - "type": "string", - "description": "Bash command to execute" - }, - "summary": { - "type": "string", - "description": "A brief explanation of what the command does" - } - }, - "required": [ - "command" - ] - } - }, "fs_read": { "name": "fs_read", "description": "Tool for reading files, directories and images. Always provide an 'operations' array.\n\nFor single operation: provide array with one element.\nFor batch operations: provide array with multiple elements.\n\nAvailable modes:\n- Line: Read lines from a file\n- Directory: List directory contents\n- Search: Search for patterns in files\n- Image: Read and process images\n\nExamples:\n1. Single: {\"operations\": [{\"mode\": \"Line\", \"path\": \"/file.txt\"}]}\n2. Batch: {\"operations\": [{\"mode\": \"Line\", \"path\": \"/file1.txt\"}, {\"mode\": \"Search\", \"path\": \"/file2.txt\", \"pattern\": \"test\"}]}",