diff --git a/README.md b/README.md index c7b7f5b..3a039c8 100644 --- a/README.md +++ b/README.md @@ -83,7 +83,9 @@ keyboard alone, and the mouse stays first-class. checks, conflicts, merge state, and provider freshness before merging with merge-commit, squash, or rebase through a GitHub-style split control. Authentication stays in the signed-in `gh` / `az` CLI; provider policies - remain enforced. Azure inline comments need iteration tracking and + remain enforced. Packaged desktop builds import the user's shell `PATH`, so + Homebrew and version-manager CLI installs work without a custom path. Azure + inline comments need iteration tracking and submit-review and other lifecycle actions are still in progress. - **Worktrees (⌘5)** — an AI-agent dashboard for every worktree with stable repo naming, branch/session labels, dirty count, ±lines, "touched 3m ago" @@ -131,7 +133,9 @@ keyboard alone, and the mouse stays first-class. unstaged changes when nothing is staged) via your ChatGPT subscription (Codex CLI, `gpt-5.6-luna`) or Claude Code CLI (`claude-sonnet-5`); Settings → AI for sign-in, provider choice, and CLI - health checks. Generation is cancellable, scans conservative sensitive-file + health checks. Packaged builds resolve these tools and their runtimes through + the user's shell `PATH`; custom paths remain available. Generation is + cancellable, scans conservative sensitive-file signals before provider launch, reports partial-context coverage, preserves the replaced draft for one-step undo, and can retry explicitly with the other provider without changing your default. Repository-family writing profiles diff --git a/ROADMAP.md b/ROADMAP.md index 723a25e..6cec193 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1659,6 +1659,14 @@ short-hash context and read-only tree actions. Opening a historical file pins Content, image/SVG previews, Markdown, and repo-relative Markdown images to the same revision instead of accidentally reading the mutable working tree. +**Packaged CLI discovery fixed (2026-07-15):** Desktop launches now recover +the user's interactive login-shell PATH once in the background and pass the +merged value directly to hosted-provider and AI CLI children. Release builds +therefore find Homebrew, local-bin, and version-manager installs of +`gh`/`az`/`codex`/`claude`, including npm launchers whose `env node` runtime +also depends on PATH, while executable lookup remains outside the untrusted +repository working directory. + --- ## 1.1+ — Post-1.0 diff --git a/TASKS.md b/TASKS.md index 8e45c07..c69a924 100644 --- a/TASKS.md +++ b/TASKS.md @@ -1782,6 +1782,11 @@ extraction above as prerequisite. **Do not start before 1.0 ships** stops per-call console flashes in the release build; CommitBar surfaces suggest failures inline as "Suggestion failed: …" instead of a silently disabled sparkle / mislabeled "Commit failed:") +- ☑ Desktop-launch CLI environment restoration (`path_env.rs` asynchronously + captures the interactive login-shell PATH once and retains conventional + Homebrew/local-bin fallbacks; shared canonical lookup, blank-override + normalization, and child PATH propagation make `gh`/`az`/`codex`/`claude` + plus npm runtimes available in packaged apps without searching the repo) - ☑ Broken vendor-CLI installs stay distinct from signed-out sessions (`AiProviderStatus.error`, auth-failure classification, and `--version` login preflight prevent false “browser opened” messages) diff --git a/crates/strand-tauri/src/ai/bin.rs b/crates/strand-tauri/src/ai/bin.rs index cd96e1a..a43f177 100644 --- a/crates/strand-tauri/src/ai/bin.rs +++ b/crates/strand-tauri/src/ai/bin.rs @@ -44,7 +44,7 @@ pub fn resolve_claude(override_path: Option<&str>) -> Option { } pub(crate) fn resolve_cli(default_name: &str, override_path: Option<&str>) -> Option { - if let Some(p) = override_path { + if let Some(p) = override_path.filter(|path| !path.trim().is_empty()) { let path = PathBuf::from(p); return canonical_spawnable(&path); } @@ -76,7 +76,7 @@ fn canonical_spawnable(path: &Path) -> Option { } fn which_on_path(name: &str) -> Option { - let path_var = std::env::var_os("PATH")?; + let path_var = crate::path_env::effective_path()?; let dirs: Vec = std::env::split_paths(&path_var).collect(); find_in_dirs(name, &dirs) } @@ -128,12 +128,19 @@ pub(crate) fn base_command(program: &Path, hide_console: bool) -> Command { const CREATE_NO_WINDOW: u32 = 0x0800_0000; cmd.creation_flags(CREATE_NO_WINDOW); } + if let Some(path) = crate::path_env::effective_path() { + cmd.env("PATH", path); + } cmd } #[cfg(not(windows))] { let _ = hide_console; - Command::new(program) + let mut cmd = Command::new(program); + if let Some(path) = crate::path_env::effective_path() { + cmd.env("PATH", path); + } + cmd } } @@ -463,6 +470,12 @@ mod tests { assert_eq!(resolved, std::fs::canonicalize(path).ok()); } + #[cfg(unix)] + #[test] + fn blank_override_falls_back_to_path() { + assert!(resolve_cli("sh", Some(" \t ")).is_some()); + } + #[cfg(unix)] #[test] fn override_rejects_non_executable_file() { diff --git a/crates/strand-tauri/src/main.rs b/crates/strand-tauri/src/main.rs index 2651030..c50ac07 100644 --- a/crates/strand-tauri/src/main.rs +++ b/crates/strand-tauri/src/main.rs @@ -2,6 +2,7 @@ mod ai; mod commands; +mod path_env; mod pull_requests; mod state; @@ -43,6 +44,11 @@ fn main() { .unwrap_or_else(|_| tracing_subscriber::EnvFilter::new("strand=info,strand_core=info")); tracing_subscriber::fmt().with_env_filter(filter).init(); + // GUI launchers commonly omit the user's shell PATH. Resolve it in the + // background so provider and AI CLIs are ready without delaying the first + // window; child commands receive it explicitly (see `path_env`). + path_env::warm_up(); + // Process-global git engine setup (disables git2's owner validation so it // opens the same repos gix already does — see strand_core::init). Must run // before the Tauri runtime spawns any command thread. diff --git a/crates/strand-tauri/src/path_env.rs b/crates/strand-tauri/src/path_env.rs new file mode 100644 index 0000000..738e38b --- /dev/null +++ b/crates/strand-tauri/src/path_env.rs @@ -0,0 +1,293 @@ +//! Recover the user's shell `PATH` for desktop-launched CLI processes. +//! +//! macOS LaunchServices (and some Linux desktop launchers) start GUI apps with +//! a minimal environment that omits Homebrew, npm version managers, and +//! `~/.local/bin`. Resolve the interactive login shell once, then pass the +//! merged path directly to child processes. We deliberately do not mutate the +//! process environment after Tauri has started its worker threads. + +use std::ffi::{OsStr, OsString}; +use std::sync::OnceLock; + +static EFFECTIVE_PATH: OnceLock> = OnceLock::new(); + +/// Start shell discovery without delaying the first window. A CLI command that +/// arrives before discovery finishes waits on the same `OnceLock` rather than +/// launching a second shell. +pub fn warm_up() { + #[cfg(unix)] + { + let _ = std::thread::Builder::new() + .name("strand-shell-path".into()) + .spawn(|| { + let _ = effective_path(); + }); + } +} + +/// Path to apply to CLI children and use for executable discovery. +pub fn effective_path() -> Option<&'static OsStr> { + EFFECTIVE_PATH + .get_or_init(resolve_effective_path) + .as_deref() +} + +fn resolve_effective_path() -> Option { + let inherited = std::env::var_os("PATH"); + #[cfg(unix)] + { + let conventional = conventional_desktop_path(); + let fallback = merge_paths(conventional.as_deref(), inherited.as_deref()); + return merge_paths(shell_path().as_deref(), fallback.as_deref()); + } + #[cfg(not(unix))] + inherited +} + +#[cfg(unix)] +fn conventional_desktop_path() -> Option { + let mut dirs = Vec::new(); + #[cfg(target_os = "macos")] + dirs.extend( + ["/opt/homebrew/bin", "/opt/homebrew/sbin", "/usr/local/bin"] + .into_iter() + .map(std::path::PathBuf::from), + ); + if let Some(home) = std::env::var_os("HOME") { + let home = std::path::PathBuf::from(home); + dirs.push(home.join(".local/bin")); + dirs.push(home.join(".cargo/bin")); + } + (!dirs.is_empty()) + .then(|| std::env::join_paths(dirs).ok()) + .flatten() +} + +fn merge_paths(preferred: Option<&OsStr>, fallback: Option<&OsStr>) -> Option { + let mut dirs = Vec::new(); + for path in [preferred, fallback].into_iter().flatten() { + for dir in std::env::split_paths(path) { + // An empty PATH component means the current directory. Provider + // lookup must never select an executable from an opened repo. + if !dir.as_os_str().is_empty() && !dirs.contains(&dir) { + dirs.push(dir); + } + } + } + (!dirs.is_empty()) + .then(|| std::env::join_paths(dirs).ok()) + .flatten() +} + +#[cfg(unix)] +fn shell_path() -> Option { + let shell = std::env::var_os("SHELL").unwrap_or_else(|| { + #[cfg(target_os = "macos")] + { + OsString::from("/bin/zsh") + } + #[cfg(not(target_os = "macos"))] + { + OsString::from("/bin/sh") + } + }); + capture_shell_path( + OsStr::new(&shell), + std::env::var_os("HOME").as_deref(), + std::time::Duration::from_secs(10), + ) +} + +#[cfg(unix)] +fn capture_shell_path( + shell: &OsStr, + home: Option<&OsStr>, + timeout: std::time::Duration, +) -> Option { + use std::io::Read; + use std::os::unix::ffi::{OsStrExt, OsStringExt}; + use std::os::unix::process::CommandExt; + use std::process::{Command, Stdio}; + use std::time::{Duration, Instant}; + + const MARKER: &[u8] = b"__STRAND_PATH__"; + const MAX_OUTPUT: usize = 1_048_576; + let shell = std::fs::canonicalize(shell).ok()?; + if !shell.is_file() { + return None; + } + + let mut command = Command::new(shell); + command + .args(["-ilc", "/usr/bin/printf '__STRAND_PATH__%s\\n' \"$PATH\""]) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()) + .process_group(0); + if let Some(home) = home { + command.current_dir(home); + } + + let mut child = command.spawn().ok()?; + let mut stdout = child.stdout.take()?; + let (output_tx, output_rx) = std::sync::mpsc::sync_channel(1); + std::thread::spawn(move || { + let mut retained = Vec::new(); + let mut chunk = [0u8; 8192]; + loop { + match stdout.read(&mut chunk) { + Ok(0) | Err(_) => break, + Ok(read) if retained.len() < MAX_OUTPUT => { + let remaining = MAX_OUTPUT - retained.len(); + retained.extend_from_slice(&chunk[..read.min(remaining)]); + } + Ok(_) => {} + } + } + let _ = output_tx.send(retained); + }); + + let deadline = Instant::now() + timeout; + let succeeded = loop { + match child.try_wait() { + Ok(Some(status)) => break status.success(), + Ok(None) if Instant::now() < deadline => { + std::thread::sleep(Duration::from_millis(25)); + } + Ok(None) | Err(_) => { + // SAFETY: the shell was created as the leader of this process + // group, so a negative PID terminates startup-script children + // as well as the shell itself. + unsafe { libc::kill(-(child.id() as i32), libc::SIGKILL) }; + let _ = child.wait(); + break false; + } + } + }; + let output = match output_rx.recv_timeout(Duration::from_millis(250)) { + Ok(output) => output, + Err(_) => { + // A startup script may leave a background child holding stdout + // open after the shell exits. It belongs to the probe's process + // group, so terminate it rather than blocking CLI discovery. + unsafe { libc::kill(-(child.id() as i32), libc::SIGKILL) }; + output_rx.recv_timeout(Duration::from_millis(250)).ok()? + } + }; + if !succeeded { + return None; + } + + output + .split(|byte| *byte == b'\n') + .rev() + .find_map(|line| line.strip_prefix(MARKER)) + .filter(|path| !path.is_empty()) + .map(|path| OsString::from_vec(path.to_vec())) + .filter(|path| !path.as_os_str().as_bytes().contains(&0)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn preferred_shell_path_wins_and_fallback_entries_are_kept_once() { + let separator = if cfg!(windows) { ";" } else { ":" }; + let preferred = OsString::from(format!("/shell/bin{separator}/shared/bin")); + let fallback = OsString::from(format!("/system/bin{separator}/shared/bin")); + let merged = merge_paths(Some(&preferred), Some(&fallback)).unwrap(); + let dirs: Vec<_> = std::env::split_paths(&merged).collect(); + assert_eq!( + dirs, + ["/shell/bin", "/shared/bin", "/system/bin"] + .into_iter() + .map(std::path::PathBuf::from) + .collect::>() + ); + } + + #[test] + fn inherited_path_survives_when_shell_discovery_is_unavailable() { + let inherited = OsStr::new(if cfg!(windows) { + r"C:\\Windows\\System32" + } else { + "/usr/bin:/bin" + }); + assert_eq!( + merge_paths(None, Some(inherited)).as_deref(), + Some(inherited) + ); + } + + #[test] + fn empty_path_components_never_enable_current_directory_lookup() { + let separator = if cfg!(windows) { ";" } else { ":" }; + let path = OsString::from(format!("{separator}/usr/bin{separator}")); + let merged = merge_paths(Some(&path), None).unwrap(); + assert_eq!( + std::env::split_paths(&merged).collect::>(), + vec![std::path::PathBuf::from("/usr/bin")] + ); + } + + #[cfg(target_os = "macos")] + #[test] + fn conventional_macos_path_includes_homebrew() { + let path = conventional_desktop_path().unwrap(); + let dirs: Vec<_> = std::env::split_paths(&path).collect(); + assert!(dirs.contains(&std::path::PathBuf::from("/opt/homebrew/bin"))); + assert!(dirs.contains(&std::path::PathBuf::from("/usr/local/bin"))); + } + + #[cfg(unix)] + #[test] + fn shell_capture_accepts_noisy_startup_output() { + use std::io::Write; + use std::os::unix::fs::PermissionsExt; + + let dir = tempfile::tempdir().unwrap(); + let shell = dir.path().join("test-shell"); + let mut file = std::fs::File::create(&shell).unwrap(); + file.write_all( + b"#!/bin/sh\nprintf 'startup noise\\n__STRAND_PATH__/shell/bin:/usr/bin\\nexit noise\\n'\n", + ) + .unwrap(); + let mut permissions = file.metadata().unwrap().permissions(); + permissions.set_mode(0o700); + std::fs::set_permissions(&shell, permissions).unwrap(); + drop(file); + + let path = capture_shell_path( + shell.as_os_str(), + Some(dir.path().as_os_str()), + std::time::Duration::from_secs(1), + ); + assert_eq!(path.as_deref(), Some(OsStr::new("/shell/bin:/usr/bin"))); + } + + #[cfg(unix)] + #[test] + fn shell_capture_times_out_and_kills_startup_children() { + use std::io::Write; + use std::os::unix::fs::PermissionsExt; + + let dir = tempfile::tempdir().unwrap(); + let shell = dir.path().join("slow-shell"); + let mut file = std::fs::File::create(&shell).unwrap(); + file.write_all(b"#!/bin/sh\nsleep 30\n").unwrap(); + let mut permissions = file.metadata().unwrap().permissions(); + permissions.set_mode(0o700); + std::fs::set_permissions(&shell, permissions).unwrap(); + drop(file); + + let started = std::time::Instant::now(); + let path = capture_shell_path( + shell.as_os_str(), + Some(dir.path().as_os_str()), + std::time::Duration::from_millis(50), + ); + assert!(path.is_none()); + assert!(started.elapsed() < std::time::Duration::from_secs(2)); + } +} diff --git a/docs/learnings.md b/docs/learnings.md index b3cbb44..6e8c86a 100644 --- a/docs/learnings.md +++ b/docs/learnings.md @@ -6,6 +6,34 @@ that future work (yours or another agent's) needs to respect. --- +## Desktop CLI children need the user's shell PATH, including dependencies + +**Rule.** Packaged GUI apps must not use their inherited process `PATH` as the +only source for hosted-provider and AI CLIs. Resolve the user's interactive +login-shell PATH once, off the launch hot path, merge it ahead of inherited +fallback directories, and pass it explicitly to each child. Keep executable +lookup canonical and complete it before setting an untrusted repository as the +child's working directory. + +**Why.** Finder/LaunchServices and Linux desktop launchers commonly omit +Homebrew, `~/.local/bin`, and version-manager directories. That made installed +`gh`, `codex`, and `claude` binaries appear missing in release builds. Resolving +only the launcher path is insufficient: npm shims commonly use +`#!/usr/bin/env node`, so the launched child also needs the recovered PATH or it +still fails at runtime. Searching after switching to a repository working +directory is unsafe on Windows because `CreateProcess` may select a +repository-owned executable. + +**How to apply.** `strand-tauri/path_env.rs` owns the bounded, background shell +capture and conventional Homebrew/local-bin fallbacks when shell startup fails. +`ai/bin.rs::resolve_cli` treats blank custom overrides as unset, searches that +effective path, and `base_command` applies it to GitHub, Azure, Codex, and +Claude children. Reuse that boundary for new hosted-provider CLIs; do not add +ad hoc `which` calls or mutate the process environment after Tauri worker +threads have started. + +--- + ## UI must be responsive and resizable **Rule.** The app needs to feel responsive at any window size, and every diff --git a/website/docs/pull-requests.md b/website/docs/pull-requests.md index 3ce78d7..8dc1cbd 100644 --- a/website/docs/pull-requests.md +++ b/website/docs/pull-requests.md @@ -13,6 +13,10 @@ or stores its access token: - GitHub requires [GitHub CLI](https://cli.github.com/) and `gh auth login`. - Azure DevOps requires Azure CLI, the `azure-devops` extension, and `az login`. +Packaged desktop builds import the interactive login-shell `PATH`, including +Homebrew, local-bin, and version-manager locations that GUI launchers normally +omit. Restart Strand after changing shell startup files or installing a CLI. + If the CLI is missing, signed out, or cannot access the repository, the view shows the provider error and the setup command. Provider calls time out after 30 seconds instead of blocking the app indefinitely. diff --git a/website/docs/settings.md b/website/docs/settings.md index 79f1346..b43c68b 100644 --- a/website/docs/settings.md +++ b/website/docs/settings.md @@ -69,6 +69,13 @@ vendor's CLI; Strand only orchestrates it. worktrees share the profile through their canonical `common_dir`; an empty profile uses recent commit subjects only. +In a packaged desktop build, PATH means the merged interactive login-shell +PATH, not only the minimal environment supplied by the desktop launcher. This +finds Homebrew, local-bin, and version-manager installs and also supplies +runtime commands such as `node` to npm-installed CLIs. Restart Strand after +changing shell startup files or installing a CLI; use the custom path only when +you need to override shell resolution. + To get a commit suggestion, stage some changes and press the sparkle button next to the commit subject field in Local Changes, use `Mod+Shift+M`, or run "Suggest commit message" from the palette. To draft a PR, use **Fill with Codex/Claude