diff --git a/plugins/worktrunk/skills/worktrunk/reference/shell-integration.md b/plugins/worktrunk/skills/worktrunk/reference/shell-integration.md index 570b83da49..b7039915ef 100644 --- a/plugins/worktrunk/skills/worktrunk/reference/shell-integration.md +++ b/plugins/worktrunk/skills/worktrunk/reference/shell-integration.md @@ -60,16 +60,22 @@ When shell integration isn't working, `wt switch` shows warnings explaining why. ### "shell integration not installed" -**Meaning**: The shell config file doesn't have the `eval "$(wt config shell init ...)"` line. +**Meaning**: The current shell's config file doesn't have the +`eval "$(wt config shell init ...)"` line. The current shell is detected from +the process tree (falling back to `$SHELL`), so this refers to the shell wt +was actually invoked from, not necessarily the login shell. **Fix**: Run `wt config shell install` or add the line manually. -### "shell requires restart" +### "shell integration installed but not active" -**Meaning**: Shell integration is configured, but the current shell session was -started before installation. The shell function isn't loaded yet. +**Meaning**: Shell integration is configured for the current shell, but the +shell function isn't loaded in this session — usually because the session was +started before installation. -**Fix**: Start a new terminal or run `source ~/.bashrc` (or equivalent). +**Fix**: Start a new terminal or run `source ~/.bashrc` (or equivalent). If +the message persists after a restart, `wt config show` reports the detected +shell, `$SHELL`, and per-shell integration status. ### "ran ./path/to/wt; shell integration wraps wt" diff --git a/skills/worktrunk/reference/shell-integration.md b/skills/worktrunk/reference/shell-integration.md index 570b83da49..b7039915ef 100644 --- a/skills/worktrunk/reference/shell-integration.md +++ b/skills/worktrunk/reference/shell-integration.md @@ -60,16 +60,22 @@ When shell integration isn't working, `wt switch` shows warnings explaining why. ### "shell integration not installed" -**Meaning**: The shell config file doesn't have the `eval "$(wt config shell init ...)"` line. +**Meaning**: The current shell's config file doesn't have the +`eval "$(wt config shell init ...)"` line. The current shell is detected from +the process tree (falling back to `$SHELL`), so this refers to the shell wt +was actually invoked from, not necessarily the login shell. **Fix**: Run `wt config shell install` or add the line manually. -### "shell requires restart" +### "shell integration installed but not active" -**Meaning**: Shell integration is configured, but the current shell session was -started before installation. The shell function isn't loaded yet. +**Meaning**: Shell integration is configured for the current shell, but the +shell function isn't loaded in this session — usually because the session was +started before installation. -**Fix**: Start a new terminal or run `source ~/.bashrc` (or equivalent). +**Fix**: Start a new terminal or run `source ~/.bashrc` (or equivalent). If +the message persists after a restart, `wt config show` reports the detected +shell, `$SHELL`, and per-shell integration status. ### "ran ./path/to/wt; shell integration wraps wt" diff --git a/src/commands/config/show.rs b/src/commands/config/show.rs index 9c9637052a..34514e78df 100644 --- a/src/commands/config/show.rs +++ b/src/commands/config/show.rs @@ -1200,11 +1200,22 @@ fn render_shell_status(out: &mut String) -> anyhow::Result<()> { } } + // Show the shell actually running wt — the process tree names the + // interactive shell even when $SHELL points at a different login shell + let ancestor = worktrunk::shell::ancestor_shell(); + if let Some(ancestor) = ancestor { + debug_lines.push(cformat!( + "Detected shell: {} (process tree)", + ancestor.name + )); + } // Show $SHELL to help diagnose rc file sourcing issues let shell_env = std::env::var("SHELL").ok().filter(|s| !s.is_empty()); if let Some(shell_env) = &shell_env { debug_lines.push(cformat!("$SHELL: {shell_env}")); - } else if let Some(detected) = worktrunk::shell::current_shell() { + } else if ancestor.is_none() + && let Some(detected) = worktrunk::shell::current_shell() + { debug_lines.push(cformat!( "Detected shell: {detected} (via PSModulePath)" )); diff --git a/src/commands/configure_shell.rs b/src/commands/configure_shell.rs index d00a1e6597..266362ffea 100644 --- a/src/commands/configure_shell.rs +++ b/src/commands/configure_shell.rs @@ -333,11 +333,13 @@ pub fn handle_configure_shell( // // We check when: // - User explicitly runs `install zsh` (they clearly want zsh integration) - // - User runs `install` (all shells) AND their $SHELL is zsh (they use zsh daily) + // - User runs `install` (all shells) AND their current shell (process + // tree, falling back to $SHELL) is zsh (they use zsh daily) // // We skip if: - // - User runs `install` but their $SHELL is bash/fish (they may be configuring - // zsh for occasional use; don't nag about their non-primary shell) + // - User runs `install` but their current shell is bash/fish (they may be + // configuring zsh for occasional use; don't nag about their non-primary + // shell) // - Zsh was already configured (AlreadyExists) - they've seen this before let zsh_was_configured = result .configured diff --git a/src/output/shell_integration.rs b/src/output/shell_integration.rs index 567dd740e2..1b164ba3fc 100644 --- a/src/output/shell_integration.rs +++ b/src/output/shell_integration.rs @@ -9,7 +9,7 @@ //! | Condition | Warning | Hint | //! |-----------|---------|------| //! | Not installed | `Worktree for X @ path, but cannot change directory — shell integration not installed` | `To enable automatic cd, run wt config shell install` | -//! | Needs restart | `Worktree for X @ path, but cannot change directory — shell requires restart` | `Restart shell to activate shell integration` | +//! | Installed, not active | `Worktree for X @ path, but cannot change directory — shell integration installed but not active` | `A shell restart usually activates shell integration; if it doesn't, ask an agent to debug with the docs @ https://worktrunk.dev/llms.txt` | //! | Explicit path | `Worktree for X @ path, but cannot change directory — ran ./wt; shell integration wraps wt` | `To change directory, run wt switch X` | //! | Git subcommand | `Worktree for X @ path, but cannot change directory — ran git wt; running through git prevents cd` | `For automatic cd, invoke directly (with the -): git-wt` | //! @@ -41,8 +41,8 @@ //! |-----------|--------| //! | Git subcommand | Return early (warning already shown) | //! | Unsupported shell | Hint: `Shell integration not yet supported for ` | -//! | $SHELL not set | Hint: `To enable automatic cd, run wt config shell install` | -//! | Current shell already installed | Hint: `Restart shell to activate shell integration` | +//! | No shell detected | Hint: `To enable automatic cd, run wt config shell install` | +//! | Current shell already installed | Hint: `A shell restart usually activates shell integration; …` | //! | `skip-shell-integration-prompt` / Non-TTY | Hint: `To enable automatic cd, run wt config shell install` | //! | TTY | Prompt: `Install shell integration? [y/N/?]` | //! @@ -53,7 +53,7 @@ //! | Reason | Meaning | //! |--------|---------| //! | `shell integration not installed` | Shell config doesn't have the `eval` line | -//! | `shell requires restart` | Shell config has `eval` line but wrapper not active | +//! | `shell integration installed but not active` | Shell config has `eval` line but wrapper not active | //! | `ran X; shell integration wraps Y` | Invoked with explicit path (e.g., `./target/debug/wt`) | //! //! Note: The git subcommand case (`ran git wt; ...`) is handled separately via [`crate::is_git_subcommand`]. @@ -64,7 +64,7 @@ use color_print::cformat; use worktrunk::config::{UserConfig, require_config_path}; use worktrunk::git::Repository; use worktrunk::path::format_path_for_display; -use worktrunk::shell::{Shell, current_shell, current_shell_name, extract_filename_from_path}; +use worktrunk::shell::{Shell, current_shell, current_shell_name}; use worktrunk::styling::{ eprintln, format_bash_with_gutter, hint_message, info_message, success_message, warning_message, }; @@ -109,16 +109,26 @@ pub(crate) fn print_shell_integration_hint(repo: &Repository) { let _ = repo.mark_hint_shown(SHELL_INTEGRATION_HINT); } -/// Hint when shell integration is installed but shell needs restart. +/// Hint shown right after installing: the wrapper cannot be active yet, so +/// the restart advice is unconditional. pub(crate) fn shell_restart_hint() -> &'static str { "Restart shell to activate shell integration" } +/// Hint when integration is installed for the current shell but the wrapper +/// didn't intercept this invocation. Hedged: shell detection can still be +/// wrong (non-interactive contexts, exotic nesting), so restart is presented +/// as the likely fix with an escape hatch, not a certainty. The escape hatch +/// points at the LLM docs index, which any agent can fetch — unlike the +/// `/worktrunk` skill, which only plugin users have. +pub(crate) fn shell_inactive_hint() -> String { + cformat!( + "A shell restart usually activates shell integration; if it doesn't, ask an agent to debug with the docs @ https://worktrunk.dev/llms.txt" + ) +} + /// Shell integration hint for unknown/unsupported shell. -fn shell_integration_unsupported_shell(shell_path: &str) -> String { - // Extract shell name from path, handling both Unix and Windows paths - // e.g., "/bin/tcsh" -> "tcsh", "C:\...\tcsh.exe" -> "tcsh" - let shell_name = extract_filename_from_path(shell_path).unwrap_or(shell_path); +fn shell_integration_unsupported_shell(shell_name: &str) -> String { format!( "Shell integration not yet supported for {shell_name} (supports bash, zsh, fish, nu, PowerShell)" ) @@ -150,10 +160,10 @@ pub(crate) fn should_show_explicit_path_hint() -> bool { /// Returns a reason string explaining why shell integration isn't working. /// See the module documentation for the complete spec of warning messages. /// -/// Checks specifically if the CURRENT shell (detected via $SHELL or PSModulePath -/// fallback) has integration configured, not just any shell. This prevents misleading -/// "shell requires restart" messages when e.g. bash has integration but the user is -/// running fish. +/// Checks specifically if the CURRENT shell (detected via the process tree, +/// falling back to $SHELL / PSModulePath) has integration configured, not just +/// any shell. This prevents misleading "restart" advice when e.g. bash has +/// integration but the user is running fish. pub(crate) fn compute_shell_warning_reason() -> String { // Check if the CURRENT shell has integration configured, not just ANY shell let is_configured = current_shell() @@ -204,7 +214,7 @@ fn compute_shell_warning_reason_inner( cformat!("ran {invoked_name}; shell integration wraps {wraps}") } } else { - "shell requires restart".to_string() + "shell integration installed but not active".to_string() } } else { "shell integration not installed".to_string() @@ -363,19 +373,19 @@ pub fn print_shell_install_result(scan_result: &crate::commands::configure_shell ); } - // Restart hint for current shell + // Restart hint for current shell. Compare Shell values, not display + // names — the detected name can be "pwsh" or "zsh-5.9", which would + // never equal the canonical "powershell"/"zsh" strings. if shells_configured_count > 0 { - let current_shell = current_shell_name(); - - let current_shell_result = current_shell.as_ref().and_then(|shell_name| { + let current_shell_configured = current_shell().is_some_and(|shell| { scan_result .configured .iter() .filter(|r| !matches!(r.action, ConfigAction::AlreadyExists)) - .find(|r| r.shell.to_string().eq_ignore_ascii_case(shell_name)) + .any(|r| r.shell == shell) }); - if current_shell_result.is_some() { + if current_shell_configured { eprintln!("{}", hint_message(shell_restart_hint())); } } @@ -405,18 +415,17 @@ pub fn prompt_shell_integration( let is_tty = std::io::stdin().is_terminal() && std::io::stderr().is_terminal(); - // Check the current shell (via $SHELL or PSModulePath fallback) - // Only prompt if current shell is supported (so they benefit immediately) - let shell_env = std::env::var("SHELL").ok(); + // Check the current shell (via the process tree, falling back to $SHELL / + // PSModulePath). Only prompt if it's supported (so they benefit immediately). if current_shell().is_none() { - match &shell_env { - Some(path) => { + match current_shell_name() { + Some(name) => { eprintln!( "{}", - hint_message(shell_integration_unsupported_shell(path)) + hint_message(shell_integration_unsupported_shell(&name)) ); } - // $SHELL not set: could be Windows PowerShell, or unusual Unix setup + // No shell detected: could be Windows PowerShell, or unusual Unix setup // Point them to manual installation (and count it as a shown install hint) None => print_shell_integration_hint(repo), } @@ -444,8 +453,8 @@ pub fn prompt_shell_integration( if current_shell_installed { // Shell integration is configured but not active for this invocation if !crate::was_invoked_with_explicit_path() { - // Invoked via PATH but wrapper isn't active - needs shell restart - eprintln!("{}", hint_message(shell_restart_hint())); + // Invoked via PATH but wrapper isn't active — a restart usually fixes it + eprintln!("{}", hint_message(shell_inactive_hint())); } // For explicit paths: no hint needed - handle_switch_output() warning already explains return Ok(false); @@ -584,15 +593,11 @@ pub fn print_shell_uninstall_result(scan_result: &UninstallScanResult, explicit_ )) ); - // Hint about restarting shell (only if current shell was affected) - let current_shell = current_shell_name(); - - let current_shell_affected = current_shell.as_ref().is_some_and(|shell_name| { - scan_result - .results - .iter() - .any(|r| r.shell.to_string().eq_ignore_ascii_case(shell_name)) - }); + // Hint about restarting shell (only if current shell was affected). + // Compare Shell values, not display names — the detected name can be + // "pwsh" or "zsh-5.9", which would never equal "powershell"/"zsh". + let current_shell_affected = + current_shell().is_some_and(|shell| scan_result.results.iter().any(|r| r.shell == shell)); if current_shell_affected { eprintln!("{}", hint_message("Restart shell to complete uninstall")); @@ -647,9 +652,9 @@ mod tests { let reason = compute_shell_warning_reason_inner(true, true, "/usr/local/bin/git-wt", "wt"); assert_snapshot!(reason, @"ran git-wt; shell integration wraps wt"); - // Shell integration configured + NOT explicit path -> "shell requires restart" + // Shell integration configured + NOT explicit path -> installed but not active let reason = compute_shell_warning_reason_inner(true, false, "wt", "wt"); - assert_eq!(reason, "shell requires restart"); + assert_eq!(reason, "shell integration installed but not active"); } #[test] diff --git a/src/shell/detection.rs b/src/shell/detection.rs index 9cea3dac56..02d018b83b 100644 --- a/src/shell/detection.rs +++ b/src/shell/detection.rs @@ -77,7 +77,7 @@ use super::paths::{home_dir_required, powershell_profile_paths}; /// detection is needed. /// /// **When binary is run directly (wrapper not active):** -/// - If detection finds integration → "restart the shell to activate" +/// - If detection finds integration → "installed but not active" (restart hint) /// - If detection misses (false negative) → "shell integration not installed" /// /// **When wrapper is active:** No warnings shown regardless of detection. diff --git a/src/shell/mod.rs b/src/shell/mod.rs index 9a6f54b409..14a28b8e1a 100644 --- a/src/shell/mod.rs +++ b/src/shell/mod.rs @@ -20,7 +20,8 @@ pub use detection::{ }; pub use paths::{completion_path, config_paths, legacy_fish_conf_d_path}; pub use utils::{ - current_shell, current_shell_name, detect_zsh_compinit, extract_filename_from_path, + AncestorShell, ancestor_shell, current_shell, current_shell_name, detect_zsh_compinit, + extract_filename_from_path, }; /// Supported shells @@ -179,7 +180,8 @@ impl Shell { /// Check if this shell has integration configured. /// /// Used for accurate warning messages that need to know about the user's - /// current shell specifically (e.g., "shell requires restart" vs "not installed"). + /// current shell specifically (e.g., "installed but not active" vs "not + /// installed"). pub fn is_shell_configured(&self, cmd: &str) -> Result { let config_paths = self.config_paths(cmd)?; diff --git a/src/shell/utils.rs b/src/shell/utils.rs index 5ddf3305b5..69e36980e3 100644 --- a/src/shell/utils.rs +++ b/src/shell/utils.rs @@ -32,63 +32,80 @@ pub fn extract_filename_from_path(path: &str) -> Option<&str> { /// Determine Shell variant from a shell name (without path or extension). /// -/// Handles versioned/prefixed binaries like `zsh-5.9` or `bash5` -/// by checking if the name starts with a known shell. +/// Handles versioned binaries like `zsh-5.9` or `bash5` by accepting a known +/// shell name followed by a non-alphabetic character. The boundary check +/// keeps unrelated commands that merely start with a shell name (`fishd`, +/// `bashtop`, `numactl`) from matching — important now that names also come +/// from the process-tree walk, not just `$SHELL`. pub fn shell_from_name(shell_name: &str) -> Option { // Try exact match first if let Ok(shell) = shell_name.parse() { return Some(shell); } - // Handle versioned/prefixed binaries (e.g., "zsh-5.9", "bash5") - // Check if shell name starts with a known shell let name_lower = shell_name.to_lowercase(); - if name_lower.starts_with("zsh") { - Some(Shell::Zsh) - } else if name_lower.starts_with("bash") { - Some(Shell::Bash) - } else if name_lower.starts_with("fish") { - Some(Shell::Fish) - } else if name_lower.starts_with("nu") { - Some(Shell::Nushell) - } else if name_lower.starts_with("pwsh") || name_lower.starts_with("powershell") { - Some(Shell::PowerShell) - } else { - None + // "nushell" precedes "nu" so its remainder isn't rejected as alphabetic. + for (prefix, shell) in [ + ("powershell", Shell::PowerShell), + ("pwsh", Shell::PowerShell), + ("nushell", Shell::Nushell), + ("nu", Shell::Nushell), + ("bash", Shell::Bash), + ("zsh", Shell::Zsh), + ("fish", Shell::Fish), + ] { + if name_matches_shell(&name_lower, prefix) { + return Some(shell); + } } + None } -/// Read `$SHELL` and extract the executable name (e.g. `/usr/bin/zsh` -> "zsh"). +/// Display name of the current shell. /// -/// Returns `None` when `$SHELL` is unset or has no extractable filename. +/// Prefers the process-tree walk (the shell wt is actually running under, +/// possibly an unsupported one like "tcsh"); falls back to the `$SHELL` +/// basename. Returns `None` when neither source names a shell. pub fn current_shell_name() -> Option { + if let Some(ancestor) = ancestor_shell() { + return Some(ancestor.name.clone()); + } + shell_name_from_env() +} + +/// Read `$SHELL` and extract the executable name (e.g. `/usr/bin/zsh` -> "zsh"). +fn shell_name_from_env() -> Option { let shell_path = std::env::var("SHELL").ok()?; extract_filename_from_path(&shell_path).map(String::from) } -/// Detect the current shell from the environment. -/// -/// Uses two strategies: -/// 1. `$SHELL` environment variable (Unix standard, also set by Git Bash on Windows) -/// 2. `PSModulePath` environment variable (indicates PowerShell on all platforms) +/// Detect the shell wt is running under. /// -/// Returns `None` if neither heuristic matches a known shell. -/// -/// Works on both Unix and Windows: -/// - Unix: `/usr/bin/bash` -> Bash -/// - Windows Git Bash: `C:\Program Files\Git\usr\bin\bash.exe` -> Bash -/// - Windows PowerShell: `PSModulePath` set -> PowerShell +/// Uses three strategies, most reliable first: +/// 1. Process-tree walk: the nearest enclosing shell process. `$SHELL` names +/// the *login* shell, which is wrong whenever the interactive shell +/// differs (bash launched from zsh, a terminal profile running fish), so +/// the process tree is consulted first. A known-but-unsupported enclosing +/// shell (e.g. tcsh) returns `None` here — falling back to `$SHELL` would +/// reintroduce the wrong-shell answer; `current_shell_name()` still names +/// it for messages. +/// 2. `$SHELL` environment variable (Unix standard, also set by Git Bash on +/// Windows), when the walk finds no shell (unsupported platform, wt +/// spawned outside any shell). +/// 3. `PSModulePath` environment variable (indicates PowerShell on all +/// platforms). On Windows this has some false positives (PSModulePath can +/// be set system-wide), but for diagnostic purposes that's acceptable — a +/// slightly less accurate message is better than "shell integration not +/// installed" when it IS installed. pub fn current_shell() -> Option { - // Primary: $SHELL (Unix standard, also set by Git Bash on Windows) - if let Some(name) = current_shell_name() { + if let Some(ancestor) = ancestor_shell() { + return ancestor.shell; + } + + if let Some(name) = shell_name_from_env() { return shell_from_name(&name); } - // Fallback: PSModulePath indicates PowerShell (set on all platforms when - // running inside PowerShell). On Windows this has some false positives - // (PSModulePath can be set system-wide), but for diagnostic purposes - // that's acceptable — a slightly less accurate message is better than - // "shell integration not installed" when it IS installed. if std::env::var_os("PSModulePath").is_some() { return Some(Shell::PowerShell); } @@ -96,6 +113,196 @@ pub fn current_shell() -> Option { None } +/// Nearest enclosing shell process, found by walking up the process tree. +#[derive(Debug, Clone)] +pub struct AncestorShell { + /// Process name as observed, login-shell `-` prefix stripped (e.g. + /// "zsh", "tcsh"). + pub name: String, + /// The parsed shell; `None` for a known-but-unsupported shell. + pub shell: Option, +} + +/// Plain POSIX script interpreters: virtually never the interactive shell on +/// modern systems, so the walk treats them as plumbing (Makefiles, `sh -c` +/// wrappers) and keeps walking toward the real shell. +const TRANSPARENT_INTERPRETERS: &[&str] = &["sh", "dash", "ash", "busybox"]; + +/// Known interactive shells wt has no integration for. One of these stops the +/// walk — it IS the enclosing shell — but reports `shell: None` rather than +/// letting a supported shell further up the tree (or `$SHELL`) claim the +/// session. +const UNSUPPORTED_SHELLS: &[&str] = &[ + "tcsh", "csh", "ksh", "mksh", "oksh", "loksh", "yash", "elvish", "xonsh", "oil", "osh", +]; + +/// True when `name` is `prefix` optionally followed by a version-ish suffix: +/// "zsh" matches "zsh", "zsh-5.9", "zsh5" — but not "zshx" ("fishd", +/// "bashtop", and "numactl" must not read as shells). +/// +/// The dash boundary is deliberately permissive so versioned binaries keep +/// matching (`zsh-5.9`, `pwsh-preview`); the cost is that a dashed tool name +/// starting with a shell name (`bash-language-server`, or its 15-char +/// `/proc` comm truncation `bash-language-s`) also classifies as that shell. +/// Acceptable: such tools don't sit in wt's interactive ancestor chain. +fn name_matches_shell(name: &str, prefix: &str) -> bool { + name.strip_prefix(prefix) + .is_some_and(|rest| !rest.chars().next().is_some_and(|c| c.is_ascii_alphabetic())) +} + +/// Nearest enclosing shell process, cached for the invocation. +/// +/// Walks up from wt's parent, passing through non-shell ancestors (`git` for +/// `git wt`, `sudo`, script runners) until it finds a known shell. Implemented +/// for Linux (`/proc//stat`) and macOS (a `ps` snapshot); elsewhere +/// (Windows, BSDs) returns `None` and callers fall back to `$SHELL` / +/// `PSModulePath` — Git Bash sets `$SHELL` itself, so Windows loses little. +/// +/// `WORKTRUNK_TEST_PARENT_SHELL` overrides the walk for tests: empty means "no +/// shell ancestor found" (integration tests run wt under a test harness whose +/// real ancestry would nondeterministically include the developer's or CI +/// runner's shell); a process name simulates finding that ancestor. +pub fn ancestor_shell() -> Option<&'static AncestorShell> { + static ANCESTOR: std::sync::OnceLock> = std::sync::OnceLock::new(); + ANCESTOR.get_or_init(detect_ancestor_shell).as_ref() +} + +fn detect_ancestor_shell() -> Option { + if let Some(value) = std::env::var_os("WORKTRUNK_TEST_PARENT_SHELL") { + let name = value.to_str()?.trim(); + if name.is_empty() { + return None; + } + return ancestor_from_name(name); + } + + #[cfg(unix)] + { + walk_ancestors(std::os::unix::process::parent_id(), process_name_and_ppid) + } + #[cfg(not(unix))] + { + None + } +} + +/// Walk up the process tree from `pid`, looking each ancestor up in +/// `lookup`, until a shell stops the walk. Bounded: deep enough for wrappers +/// (git, sudo, script runners), small enough that a cycle or exotic tree +/// can't stall the warning path. An unreadable hop ends the walk — without a +/// parent pid there is nothing to continue from — and callers fall back to +/// `$SHELL`. +#[cfg(unix)] +fn walk_ancestors( + mut pid: u32, + lookup: impl Fn(u32) -> Option<(String, u32)>, +) -> Option { + for _ in 0..16 { + if pid <= 1 { + return None; + } + let (name, ppid) = lookup(pid)?; + tracing::debug!(pid, ppid, name, "shell ancestry hop"); + if let Some(found) = ancestor_from_name(&name) { + return Some(found); + } + if ppid == pid { + return None; + } + pid = ppid; + } + None +} + +/// Classify a process name, returning the walk's result if it's a stop: +/// a supported or known-unsupported shell stops the walk; transparent +/// interpreters and non-shells return `None` (keep walking). +fn ancestor_from_name(name: &str) -> Option { + // Login shells report argv[0] with a leading dash ("-zsh"). + let name = name.strip_prefix('-').unwrap_or(name); + let lower = name.to_ascii_lowercase(); + if TRANSPARENT_INTERPRETERS.contains(&lower.as_str()) { + return None; + } + let shell = if UNSUPPORTED_SHELLS + .iter() + .any(|s| name_matches_shell(&lower, s)) + { + None + } else { + Some(shell_from_name(&lower)?) + }; + Some(AncestorShell { + name: name.to_string(), + shell, + }) +} + +/// Read (process name, parent pid) for `pid` from the OS process table. +/// +/// `/proc//stat` is `pid (comm) state ppid …`; comm may itself contain +/// spaces or parens, so parse from the last `)`. +#[cfg(target_os = "linux")] +fn process_name_and_ppid(pid: u32) -> Option<(String, u32)> { + let stat = std::fs::read_to_string(format!("/proc/{pid}/stat")).ok()?; + let open = stat.find('(')?; + let close = stat.rfind(')')?; + let name = stat.get(open + 1..close)?.to_string(); + // After ")": state, then ppid. + let ppid = stat + .get(close + 1..)? + .split_whitespace() + .nth(1)? + .parse() + .ok()?; + Some((name, ppid)) +} + +/// Read (process name, parent pid) for `pid` from a one-shot `ps` snapshot. +/// +/// macOS has no `/proc`, and the kernel's per-process `p_comm` carries the +/// executable image's name rather than the invoked name — `/bin/sh` re-execs +/// bash (via `/var/select/sh`), so an sh script's `p_comm` reads "bash" and +/// the sh-transparency rule would never fire. `ps`'s `comm` column preserves +/// argv\[0\] ("sh", "-zsh", "/bin/zsh"), which is the invoked identity the +/// classifier needs. One snapshot (a few ms, cold warning paths only) serves +/// the whole walk. +#[cfg(target_os = "macos")] +fn process_name_and_ppid(pid: u32) -> Option<(String, u32)> { + static TABLE: std::sync::OnceLock> = + std::sync::OnceLock::new(); + TABLE.get_or_init(ps_snapshot).get(&pid).cloned() +} + +/// Build the pid → (name, parent pid) table from one `ps` invocation. +#[cfg(target_os = "macos")] +fn ps_snapshot() -> std::collections::HashMap { + let Ok(output) = crate::shell_exec::Cmd::new("ps") + .args(["-Ao", "pid=,ppid=,comm="]) + .run() + else { + return std::collections::HashMap::new(); + }; + String::from_utf8_lossy(&output.stdout) + .lines() + .filter_map(|line| { + let mut fields = line.split_whitespace(); + let pid: u32 = fields.next()?.parse().ok()?; + let ppid: u32 = fields.next()?.parse().ok()?; + // comm is the remainder — argv[0] may be a path and may + // contain spaces; keep the basename. + let comm = fields.collect::>().join(" "); + let name = extract_filename_from_path(&comm)?.to_string(); + Some((pid, (name, ppid))) + }) + .collect() +} + +#[cfg(all(unix, not(any(target_os = "linux", target_os = "macos"))))] +fn process_name_and_ppid(_pid: u32) -> Option<(String, u32)> { + None +} + /// Leading flags for the interactive zsh compinit probe. /// /// `+m` (disable job control) is load-bearing — see the call site and issue @@ -291,7 +498,126 @@ mod tests { #[case::pwsh_preview("pwsh-preview", Some(Shell::PowerShell))] #[case::unknown("tcsh", None)] #[case::unknown_csh("csh", None)] + // Names that merely start with a shell name must not match — the + // process-tree walk feeds arbitrary ancestor names through here. + #[case::fish_daemon("fishd", None)] + #[case::bashtop("bashtop", None)] + #[case::numactl("numactl", None)] fn test_shell_from_name(#[case] name: &str, #[case] expected: Option) { assert_eq!(shell_from_name(name), expected); } + + /// The walk stops at the nearest shell: supported shells parse, known + /// unsupported shells stop the walk with `shell: None`, and plumbing + /// (script interpreters, non-shells) is transparent. + #[rstest] + #[case::zsh("zsh", Some(Some(Shell::Zsh)))] + #[case::login_zsh("-zsh", Some(Some(Shell::Zsh)))] + #[case::login_bash("-bash", Some(Some(Shell::Bash)))] + #[case::nix_versioned("zsh-5.9", Some(Some(Shell::Zsh)))] + #[case::tcsh("tcsh", Some(None))] + #[case::ksh("ksh", Some(None))] + #[case::ksh93("ksh93", Some(None))] + #[case::sh_transparent("sh", None)] + #[case::dash_transparent("dash", None)] + #[case::git_transparent("git", None)] + #[case::terminal_transparent("iTerm2", None)] + fn test_ancestor_from_name(#[case] name: &str, #[case] expected: Option>) { + let result = ancestor_from_name(name); + assert_eq!(result.as_ref().map(|a| a.shell), expected, "name: {name}"); + if let Some(ancestor) = result { + assert!( + !ancestor.name.starts_with('-'), + "login-shell dash must be stripped: {}", + ancestor.name + ); + } + } + + /// The ancestry walk stops at the nearest shell, passes through wrappers + /// and transparent interpreters, and terminates on init, cycles, + /// unreadable hops, and depth exhaustion. + #[cfg(unix)] + #[test] + fn test_walk_ancestors() { + use std::collections::HashMap; + + // wt's parent chain: sh (transparent) ← git (wrapper) ← -zsh (login) + let table: HashMap = HashMap::from([ + (10, ("sh".to_string(), 9)), + (9, ("git".to_string(), 8)), + (8, ("-zsh".to_string(), 1)), + ]); + let found = walk_ancestors(10, |pid| table.get(&pid).cloned()) + .expect("finds zsh through sh and git"); + assert_eq!(found.shell, Some(Shell::Zsh)); + assert_eq!(found.name, "zsh"); + + // An unsupported shell stops the walk even with a supported shell + // above it — the nearest enclosing shell owns the session. + let table: HashMap = + HashMap::from([(10, ("tcsh".to_string(), 8)), (8, ("zsh".to_string(), 1))]); + let found = walk_ancestors(10, |pid| table.get(&pid).cloned()).unwrap(); + assert_eq!(found.shell, None); + assert_eq!(found.name, "tcsh"); + + // Terminations, each with no result: starting at init, a ppid + // cycle, an unreadable hop, and a >16-deep non-shell chain. + assert!(walk_ancestors(1, |_| unreachable!("init is never looked up")).is_none()); + assert!(walk_ancestors(10, |pid| Some(("looper".to_string(), pid))).is_none()); + assert!(walk_ancestors(10, |_| None).is_none()); + assert!(walk_ancestors(u32::MAX, |pid| Some(("wrapper".to_string(), pid - 1))).is_none()); + } + + /// The OS probe reads real entries: our own pid resolves to a non-empty + /// name and our actual parent pid. + #[cfg(any(target_os = "linux", target_os = "macos"))] + #[test] + fn test_process_name_and_ppid_self() { + let (name, ppid) = process_name_and_ppid(std::process::id()) + .expect("own pid must be readable from the process table"); + assert!(!name.is_empty()); + assert_eq!(ppid, std::os::unix::process::parent_id()); + } + + /// The sh-transparency rule depends on the OS probe reporting the + /// *invoked* name for `sh` processes. On macOS the kernel's `p_comm` + /// reports the image name instead — `/bin/sh` re-execs bash, so `p_comm` + /// reads "bash" — which is why the probe there uses `ps`'s + /// argv\[0\]-derived comm. Pin that with a real child: a live `sh` must + /// probe as "sh", never as its implementation ("bash"/"dash"). + #[cfg(any(target_os = "linux", target_os = "macos"))] + #[test] + fn test_probe_reports_invoked_name_for_sh() { + // Compound command so sh doesn't exec-replace itself with `sleep`. + let mut child = Command::new("/bin/sh") + .args(["-c", "sleep 30; true"]) + .stdout(Stdio::null()) + .spawn() + .expect("spawn sh"); + + // The probe can briefly race the child's exec; poll until it settles. + // (macOS: ps_snapshot() directly — process_name_and_ppid's cached + // table may predate the child.) + let probe = |pid: u32| -> Option<(String, u32)> { + #[cfg(target_os = "macos")] + return ps_snapshot().get(&pid).cloned(); + #[cfg(target_os = "linux")] + return process_name_and_ppid(pid); + }; + let mut last = None; + for _ in 0..40 { + last = probe(child.id()); + if last.as_ref().is_some_and(|(name, _)| name == "sh") { + break; + } + std::thread::sleep(Duration::from_millis(50)); + } + let _ = child.kill(); + let _ = child.wait(); + + let (name, ppid) = last.expect("child sh must be visible to the probe"); + assert_eq!(name, "sh", "probe must report the invoked name"); + assert_eq!(ppid, std::process::id()); + } } diff --git a/src/testing/mod.rs b/src/testing/mod.rs index 35f84e8691..8dcf92f71f 100644 --- a/src/testing/mod.rs +++ b/src/testing/mod.rs @@ -272,6 +272,13 @@ pub const STATIC_TEST_ENV_VARS: &[(&str, &str)] = &[ ("WORKTRUNK_TEST_FISH_INSTALLED", "0"), ("WORKTRUNK_TEST_NUSHELL_ENV", "0"), ("WORKTRUNK_TEST_POWERSHELL_INSTALLED", "0"), + // Disable the process-tree shell walk (see `shell::ancestor_shell`): the + // real ancestry of a spawned test wt is the test harness → nextest → + // cargo → the developer's or CI runner's shell, which would leak into + // shell-detection results nondeterministically. Empty = "no shell + // ancestor found", so tests drive detection via SHELL. Tests exercising + // the walk set a shell name instead. + ("WORKTRUNK_TEST_PARENT_SHELL", ""), // Disable PowerShell auto-detection (PSModulePath / SHELL signal). // Iteration is unconditional (matches the other shells); this var only // controls `allow_create` via `should_auto_configure_powershell()` so we diff --git a/tests/integration_tests/config_show.rs b/tests/integration_tests/config_show.rs index 002a2d63e0..b24b714a52 100644 --- a/tests/integration_tests/config_show.rs +++ b/tests/integration_tests/config_show.rs @@ -2848,6 +2848,69 @@ fn test_config_show_powershell_detected_via_psmodulepath(mut repo: TestRepo, tem }); } +/// The process-tree shell wins over $SHELL in the diagnostics: with zsh as +/// the detected ancestor and bash as the login shell, config show surfaces +/// "Detected shell: zsh (process tree)" next to $SHELL, and the verify hint +/// targets zsh. +#[rstest] +fn test_config_show_detected_shell_via_process_tree(mut repo: TestRepo, temp_home: TempDir) { + repo.setup_mock_ci_tools_unauthenticated(); + + // .zshrc has integration for the shell the process tree detects + fs::write( + temp_home.path().join(".zshrc"), + r#"if command -v wt >/dev/null 2>&1; then eval "$(command wt config shell init zsh)"; fi +"#, + ) + .unwrap(); + + let settings = setup_snapshot_settings_with_home(&repo, &temp_home); + settings.bind(|| { + let mut cmd = wt_command(); + repo.configure_wt_cmd(&mut cmd); + repo.configure_mock_commands(&mut cmd); + cmd.arg("config").arg("show").current_dir(repo.root_path()); + set_temp_home_env(&mut cmd, temp_home.path()); + set_xdg_config_path(&mut cmd, temp_home.path()); + // Login shell differs from the process-tree ancestor. The override + // must come after configure_wt_cmd, which pins it to "". + cmd.env("SHELL", "/bin/bash"); + cmd.env("WORKTRUNK_TEST_PARENT_SHELL", "zsh"); + cmd.env("WORKTRUNK_TEST_COMPINIT_CONFIGURED", "1"); + + assert_cmd_snapshot!(cmd); + }); +} + +/// Without the test override, the real process-tree walk runs. Its result +/// depends on the environment (dev shell, CI runner), so this asserts only +/// that the walk completes and config show still renders — exercising the +/// production walk end-to-end rather than the override shortcut. +#[rstest] +fn test_config_show_runs_real_ancestry_walk(mut repo: TestRepo, temp_home: TempDir) { + repo.setup_mock_ci_tools_unauthenticated(); + + let mut cmd = wt_command(); + repo.configure_wt_cmd(&mut cmd); + repo.configure_mock_commands(&mut cmd); + cmd.arg("config").arg("show").current_dir(repo.root_path()); + set_temp_home_env(&mut cmd, temp_home.path()); + set_xdg_config_path(&mut cmd, temp_home.path()); + cmd.env_remove("WORKTRUNK_TEST_PARENT_SHELL"); + + let output = cmd.output().unwrap(); + assert!( + output.status.success(), + "config show should succeed with the real walk: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout.contains("Shell integration"), + "shell section should render: {stdout}" + ); +} + /// Test that deprecated [commit-generation] section shows warning and creates migration file #[rstest] fn test_deprecated_commit_generation_section_shows_warning(repo: TestRepo, temp_home: TempDir) { diff --git a/tests/integration_tests/configure_shell.rs b/tests/integration_tests/configure_shell.rs index 4912a3076d..f356280119 100644 --- a/tests/integration_tests/configure_shell.rs +++ b/tests/integration_tests/configure_shell.rs @@ -1755,6 +1755,11 @@ mod pty_tests { cmd.env("WORKTRUNK_TEST_FISH_INSTALLED", "0"); cmd.env("WORKTRUNK_TEST_POWERSHELL_INSTALLED", "0"); cmd.env("WORKTRUNK_TEST_NUSHELL_ENV", "0"); + // Disable the process-tree shell walk so detection keys on SHELL: + // the real ancestry here is the test harness and whatever shell runs + // it (zsh on a dev box, bash on CI), which would flip the zsh-only + // compinit/restart output between environments. + cmd.env("WORKTRUNK_TEST_PARENT_SHELL", ""); cmd.env("SHELL", "/bin/zsh"); // Skip the compinit probe and force the advisory to appear. The probe spawns // `zsh -ic` which triggers global zshrc configs that can produce "insecure @@ -1876,6 +1881,9 @@ mod pty_tests { cmd.env("WORKTRUNK_TEST_FISH_INSTALLED", "0"); cmd.env("WORKTRUNK_TEST_POWERSHELL_INSTALLED", "0"); cmd.env("WORKTRUNK_TEST_NUSHELL_ENV", "0"); + // Disable the process-tree shell walk so detection keys on SHELL + // (see exec_install_in_pty). + cmd.env("WORKTRUNK_TEST_PARENT_SHELL", ""); cmd.env("SHELL", "/bin/zsh"); let (output, exit_code) = exec_cmd_in_pty_prompted(cmd, &["n\n"], "[y/N"); diff --git a/tests/integration_tests/shell_integration_prompt.rs b/tests/integration_tests/shell_integration_prompt.rs index 000cd90721..e98621d5e2 100644 --- a/tests/integration_tests/shell_integration_prompt.rs +++ b/tests/integration_tests/shell_integration_prompt.rs @@ -202,6 +202,105 @@ fn test_switch_no_shell_env_shows_hint(repo: TestRepo) { ); } +/// +/// $SHELL names the login shell (zsh, with integration installed), but the +/// process tree says wt is really running under fish. Restart advice would be +/// wrong — restarting fish activates nothing — so the warning must take the +/// not-installed shape for the shell actually in use. +#[cfg(unix)] +#[rstest] +fn test_login_shell_mismatch_uses_process_tree_shell(repo: TestRepo) { + use std::os::unix::process::CommandExt; + use std::process::Stdio; + + repo.configure_shell_integration(); // writes the zsh eval line to ~/.zshrc + + let mut cmd = repo.wt_command(); + cmd.env("SHELL", "/bin/zsh"); + cmd.env("WORKTRUNK_TEST_PARENT_SHELL", "fish"); + cmd.arg0("wt"); // PATH-style invocation, not the explicit-path warning + + let output = cmd + .args(["switch", "--create", "feature"]) + .stdin(Stdio::piped()) + .output() + .unwrap(); + + assert!(output.status.success(), "Switch should succeed"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("shell integration not installed"), + "The actual shell (fish) has no integration: {stderr}" + ); + assert!( + !stderr.contains("restart"), + "Restart advice targets the wrong shell here: {stderr}" + ); +} + +/// +/// The process tree names a supported shell whose integration IS installed; +/// $SHELL points at a different, unconfigured shell. The process tree wins: +/// the warning takes the installed-but-not-active shape with the hedged +/// restart hint. +#[cfg(unix)] +#[rstest] +fn test_process_tree_shell_configured_shows_restart_hint(repo: TestRepo) { + use std::os::unix::process::CommandExt; + use std::process::Stdio; + + repo.configure_shell_integration(); // writes the zsh eval line to ~/.zshrc + + let mut cmd = repo.wt_command(); + cmd.env("SHELL", "/bin/bash"); // login shell without integration + cmd.env("WORKTRUNK_TEST_PARENT_SHELL", "zsh"); + cmd.arg0("wt"); + + let output = cmd + .args(["switch", "--create", "feature"]) + .stdin(Stdio::piped()) + .output() + .unwrap(); + + assert!(output.status.success(), "Switch should succeed"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("shell integration installed but not active"), + "zsh (from the process tree) has integration: {stderr}" + ); + assert!( + stderr.contains("A shell restart usually activates shell integration"), + "Should show the hedged restart hint: {stderr}" + ); +} + +/// +/// The process tree names a known-but-unsupported shell (tcsh); $SHELL points +/// at a supported one. The unsupported-shell hint must name the shell actually +/// in use rather than trusting $SHELL. +#[cfg(unix)] +#[rstest] +fn test_process_tree_unsupported_shell_overrides_shell_env(repo: TestRepo) { + use std::process::Stdio; + + let mut cmd = repo.wt_command(); + cmd.env("SHELL", "/bin/zsh"); + cmd.env("WORKTRUNK_TEST_PARENT_SHELL", "tcsh"); + + let output = cmd + .args(["switch", "--create", "feature"]) + .stdin(Stdio::piped()) + .output() + .unwrap(); + + assert!(output.status.success(), "Switch should succeed"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("not yet supported for tcsh"), + "Should name the shell from the process tree: {stderr}" + ); +} + // PTY-based tests for interactive scenarios #[cfg(all(unix, feature = "shell-integration-tests"))] mod pty_tests { diff --git a/tests/snapshots/integration__integration_tests__config_show__config_show_detected_shell_via_process_tree.snap b/tests/snapshots/integration__integration_tests__config_show__config_show_detected_shell_via_process_tree.snap new file mode 100644 index 0000000000..205e62a7c4 --- /dev/null +++ b/tests/snapshots/integration__integration_tests__config_show__config_show_detected_shell_via_process_tree.snap @@ -0,0 +1,73 @@ +--- +source: tests/integration_tests/config_show.rs +info: + program: wt + args: + - config + - show + env: + APPDATA: "[TEST_CONFIG_HOME]" + CLAUDE_CONFIG_DIR: "[TEST_CLAUDE_CONFIG]" + CLICOLOR_FORCE: "1" + COLUMNS: "500" + GIT_AUTHOR_DATE: "2025-01-01T00:00:00Z" + GIT_COMMITTER_DATE: "2025-01-01T00:00:00Z" + GIT_CONFIG_GLOBAL: "[TEST_GIT_CONFIG]" + GIT_CONFIG_SYSTEM: /dev/null + GIT_TERMINAL_PROMPT: "0" + HOME: "[TEST_HOME]" + LANG: C + LC_ALL: C + LLVM_PROFILE_FILE: "[LLVM_PROFILE_FILE]" + MOCK_CONFIG_DIR: "[MOCK_CONFIG_DIR]" + OPENCODE_CONFIG_DIR: "[TEST_OPENCODE_CONFIG]" + PATH: "[PATH]" + SHELL: /bin/bash + TERM: alacritty + USERPROFILE: "[TEST_HOME]" + WORKTRUNK_APPROVALS_PATH: "[TEST_APPROVALS]" + WORKTRUNK_CONFIG_PATH: "[TEST_CONFIG]" + WORKTRUNK_SYSTEM_CONFIG_PATH: "[TEST_SYSTEM_CONFIG]" + WORKTRUNK_TEST_BASH_INSTALLED: "0" + WORKTRUNK_TEST_CLAUDE_INSTALLED: "0" + WORKTRUNK_TEST_CODEX_INSTALLED: "0" + WORKTRUNK_TEST_COMPINIT_CONFIGURED: "1" + WORKTRUNK_TEST_DELAYED_STREAM_MS: "-1" + WORKTRUNK_TEST_EPOCH: "1735776000" + WORKTRUNK_TEST_FISH_INSTALLED: "0" + WORKTRUNK_TEST_GEMINI_INSTALLED: "0" + WORKTRUNK_TEST_NUSHELL_ENV: "0" + WORKTRUNK_TEST_OPENCODE_INSTALLED: "0" + WORKTRUNK_TEST_PARENT_SHELL: zsh + WORKTRUNK_TEST_POWERSHELL_ENV: "0" + WORKTRUNK_TEST_POWERSHELL_INSTALLED: "0" + WORKTRUNK_TEST_SKIP_URL_HEALTH_CHECK: "1" + WORKTRUNK_TEST_ZSH_INSTALLED: "0" + XDG_CONFIG_HOME: "[TEST_CONFIG_HOME]" +--- +success: true +exit_code: 0 +----- stdout ----- +USER CONFIG @ ~/.config/worktrunk/config.toml +↳ Not found; to create one, run wt config create + +PROJECT CONFIG @ _REPO_/.config/wt.toml +○ Identifier: ../origin +↳ Not found + +SHELL INTEGRATION +▲ Shell integration not active +  Invoked as: [PROJECT_ROOT]/target/[BUILD_MODE]/wt +  Detected shell: zsh (process tree) +  $SHELL: /bin/bash + +○ zsh: Already configured shell extension & completions @ ~/.zshrc:1 +  if command -v wt >/dev/null 2>&1; then eval "$(command wt config shell init zsh)"; fi +↳ To verify wrapper loaded: type wt + +OTHER +○ wt: [VERSION] +○ git: [VERSION] +○ Hyperlinks: inactive + +----- stderr -----