feat(shell_dispatcher): harden PowerShell detection and add execution logging
- find_exe(): fall back to known install dirs when PATH lookup fails (C:\Program Files\PowerShell\7, System32\WindowsPowerShell\v1.0) - Encoding prefix: use idiomatic [Console]::OutputEncoding for PowerShell instead of cmd.exe chcp 65001 >NUL - Execution logging: write exec via <ShellKind> entries to SHELL_DISPATCHER_LOG when set - System prompt: use ShellDispatcher detection instead of raw $SHELL so model knows it has PowerShell and generates native cmdlets - display_command(): strip PowerShell encoding prefix for display - Add tests for find_exe PATH + known-dir fallback Refs #1779
This commit is contained in:
committed by
Hunter Bown
parent
29625945de
commit
6e5d8ddf01
@@ -142,7 +142,10 @@ for the current turn."
|
||||
fn render_environment_block(workspace: &Path, locale_tag: &str) -> String {
|
||||
let deepseek_version = env!("CARGO_PKG_VERSION");
|
||||
let platform = std::env::consts::OS;
|
||||
let shell = std::env::var("SHELL").unwrap_or_else(|_| "unknown".to_string());
|
||||
let shell = crate::shell_dispatcher::global_dispatcher()
|
||||
.kind()
|
||||
.binary()
|
||||
.to_string();
|
||||
let pwd = workspace.display();
|
||||
|
||||
format!(
|
||||
|
||||
@@ -91,10 +91,13 @@ impl CommandSpec {
|
||||
|
||||
#[cfg(windows)]
|
||||
let (program, args) = {
|
||||
// Force UTF-8 output on Windows. chcp is cmd-compatible;
|
||||
// PowerShell uses semicolons instead of &. See issue #982.
|
||||
let separator = if kind.is_powershell() { ";" } else { "&" };
|
||||
let cmd = format!("chcp 65001 >NUL {separator} {command}");
|
||||
// Force UTF-8 output. cmd.exe uses chcp; PowerShell sets the
|
||||
// console output encoding directly. See issue #982.
|
||||
let cmd = if kind.is_powershell() {
|
||||
format!("[Console]::OutputEncoding = [System.Text.Encoding]::UTF8; {command}")
|
||||
} else {
|
||||
format!("chcp 65001 >NUL & {command}")
|
||||
};
|
||||
dispatcher.build_command_parts(&cmd)
|
||||
};
|
||||
#[cfg(not(windows))]
|
||||
@@ -163,6 +166,17 @@ impl CommandSpec {
|
||||
raw.strip_prefix("chcp 65001 >NUL & ")
|
||||
.unwrap_or(raw)
|
||||
.to_string()
|
||||
} else if (self.program.eq_ignore_ascii_case("pwsh")
|
||||
|| self.program.eq_ignore_ascii_case("powershell"))
|
||||
&& self.args.len() >= 3
|
||||
&& self.args[0].eq_ignore_ascii_case("-NoProfile")
|
||||
&& self.args[1].eq_ignore_ascii_case("-Command")
|
||||
{
|
||||
// Strip the PowerShell encoding prefix.
|
||||
let raw = &self.args[2];
|
||||
raw.strip_prefix("[Console]::OutputEncoding = [System.Text.Encoding]::UTF8; ")
|
||||
.unwrap_or(raw)
|
||||
.to_string()
|
||||
} else {
|
||||
// For other commands, join program and args
|
||||
let mut parts = vec![self.program.clone()];
|
||||
|
||||
@@ -15,7 +15,13 @@
|
||||
//! crossterm raw-mode so the TUI input pipeline is not broken after a
|
||||
//! child process exits (issue #1690).
|
||||
|
||||
use std::fs::OpenOptions;
|
||||
use std::io::Write;
|
||||
use std::path::Path;
|
||||
use std::process::Command;
|
||||
use std::sync::Mutex;
|
||||
|
||||
static LOG_MUTEX: Mutex<()> = Mutex::new(());
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Shell kind
|
||||
@@ -115,9 +121,51 @@ impl ShellDispatcher {
|
||||
/// 2. `/bin/sh` fallback.
|
||||
pub fn detect() -> Self {
|
||||
let kind = Self::detect_shell();
|
||||
Self::log_startup(&kind);
|
||||
ShellDispatcher { kind }
|
||||
}
|
||||
|
||||
/// Log a shell execution line when `SHELL_DISPATCHER_LOG` is set.
|
||||
pub fn log_exec(command: &str) {
|
||||
if let Ok(path) = std::env::var("SHELL_DISPATCHER_LOG") {
|
||||
let _ = Self::append_log_static(&path, command);
|
||||
}
|
||||
}
|
||||
|
||||
fn log_startup(kind: &ShellKind) {
|
||||
let _lock = LOG_MUTEX.lock();
|
||||
if let Ok(path) = std::env::var("SHELL_DISPATCHER_LOG") {
|
||||
let init_line = format!(
|
||||
"--- ShellDispatcher log started pid={} ---\n",
|
||||
std::process::id()
|
||||
);
|
||||
let _ = Self::append_log(&path, &init_line);
|
||||
let detect_line = format!("[{}] detect: {kind:?}\n", now_iso());
|
||||
let _ = Self::append_log(&path, &detect_line);
|
||||
}
|
||||
}
|
||||
|
||||
fn append_log(path: &str, line: &str) -> std::io::Result<()> {
|
||||
let mut file = OpenOptions::new()
|
||||
.create(true)
|
||||
.append(true)
|
||||
.open(Path::new(path))?;
|
||||
file.write_all(line.as_bytes())?;
|
||||
file.flush()
|
||||
}
|
||||
|
||||
fn append_log_static(path: &str, command: &str) -> std::io::Result<()> {
|
||||
// Resolve kind outside the lock — `global_dispatcher()` may trigger
|
||||
// `detect()` which calls `log_startup()` which also acquires the mutex.
|
||||
let kind = global_dispatcher().kind();
|
||||
let _lock = LOG_MUTEX.lock();
|
||||
let line = format!(
|
||||
"[{}] exec via {kind:?}: {command}\n", now_iso()
|
||||
);
|
||||
Self::append_log(path, &line)
|
||||
}
|
||||
|
||||
|
||||
/// The detected shell kind.
|
||||
pub fn kind(&self) -> &ShellKind {
|
||||
&self.kind
|
||||
@@ -180,6 +228,18 @@ impl ShellDispatcher {
|
||||
) -> Result<String, anyhow::Error> {
|
||||
use anyhow::Context;
|
||||
|
||||
// Log the execution
|
||||
{
|
||||
let _lock = LOG_MUTEX.lock();
|
||||
if let Ok(path) = std::env::var("SHELL_DISPATCHER_LOG") {
|
||||
let kind = self.kind();
|
||||
let line = format!(
|
||||
"[{}] exec via {kind:?}: {shell_command}\n", now_iso()
|
||||
);
|
||||
let _ = Self::append_log(&path, &line);
|
||||
}
|
||||
}
|
||||
|
||||
// Disable raw mode; guard restores it even on `?` early return.
|
||||
let _ = crossterm::terminal::disable_raw_mode();
|
||||
struct FgRawModeGuard;
|
||||
@@ -233,10 +293,10 @@ impl ShellDispatcher {
|
||||
|
||||
#[cfg(windows)]
|
||||
{
|
||||
if Self::binary_on_path("pwsh.exe") {
|
||||
if Self::find_exe("pwsh.exe") {
|
||||
return ShellKind::Pwsh;
|
||||
}
|
||||
if Self::binary_on_path("powershell.exe") {
|
||||
if Self::find_exe("powershell.exe") {
|
||||
return ShellKind::WindowsPowerShell;
|
||||
}
|
||||
return ShellKind::Cmd;
|
||||
@@ -248,6 +308,19 @@ impl ShellDispatcher {
|
||||
}
|
||||
}
|
||||
|
||||
/// Check PATH first, then fall back to well-known install directories.
|
||||
fn find_exe(name: &str) -> bool {
|
||||
if Self::binary_on_path(name) {
|
||||
return true;
|
||||
}
|
||||
// Well-known install locations (order by preference).
|
||||
let known_dirs: &[&str] = &[
|
||||
r"C:\Program Files\PowerShell\7",
|
||||
r"C:\Windows\System32\WindowsPowerShell\v1.0",
|
||||
];
|
||||
known_dirs.iter().any(|dir| std::path::Path::new(dir).join(name).is_file())
|
||||
}
|
||||
|
||||
fn binary_on_path(name: &str) -> bool {
|
||||
std::env::var_os("PATH")
|
||||
.map(|path| {
|
||||
@@ -260,6 +333,12 @@ impl ShellDispatcher {
|
||||
}
|
||||
}
|
||||
|
||||
// -- Helpers ---------------------------------------------------------------
|
||||
|
||||
fn now_iso() -> String {
|
||||
chrono::Utc::now().format("%Y-%m-%dT%H:%M:%S%.3f").to_string()
|
||||
}
|
||||
|
||||
/// Global dispatcher instance, detected once at startup.
|
||||
///
|
||||
/// Any code path that needs to spawn a shell command can use
|
||||
@@ -388,6 +467,29 @@ mod tests {
|
||||
assert!(args.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn find_exe_finds_cmd_on_path() {
|
||||
// cmd.exe is always on PATH on Windows.
|
||||
assert!(ShellDispatcher::find_exe("cmd.exe"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn find_exe_rejects_nonexistent_binary() {
|
||||
assert!(!ShellDispatcher::find_exe("nonexistent_xyz_12345.exe"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn find_exe_falls_back_to_known_dirs() {
|
||||
// Verify the known-dirs fallback path actually exists on this system.
|
||||
let ps_path = r"C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe";
|
||||
if std::path::Path::new(ps_path).is_file() {
|
||||
// The fallback directory exists — find_exe should locate it.
|
||||
assert!(ShellDispatcher::find_exe("powershell.exe"));
|
||||
} else {
|
||||
eprintln!("Skipping: {ps_path} not present on this system");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn custom_shell_uses_provided_binary_and_flag() {
|
||||
let kind = ShellKind::Custom {
|
||||
|
||||
@@ -731,6 +731,9 @@ impl ShellManager {
|
||||
policy_override: Option<ExecutionSandboxPolicy>,
|
||||
extra_env: HashMap<String, String>,
|
||||
) -> Result<ShellResult> {
|
||||
// Log execution via ShellDispatcher when SHELL_DISPATCHER_LOG is set.
|
||||
crate::shell_dispatcher::ShellDispatcher::log_exec(command);
|
||||
|
||||
let work_dir = working_dir.map_or_else(|| self.default_workspace.clone(), PathBuf::from);
|
||||
|
||||
// Clamp timeout to max 10 minutes (600000ms)
|
||||
@@ -794,6 +797,8 @@ impl ShellManager {
|
||||
policy_override: Option<ExecutionSandboxPolicy>,
|
||||
extra_env: HashMap<String, String>,
|
||||
) -> Result<ShellResult> {
|
||||
crate::shell_dispatcher::ShellDispatcher::log_exec(command);
|
||||
|
||||
let work_dir = working_dir.map_or_else(|| self.default_workspace.clone(), PathBuf::from);
|
||||
|
||||
let timeout_ms = timeout_ms.clamp(1000, 600_000);
|
||||
|
||||
Reference in New Issue
Block a user