From 4d6717ac0435ecc2788c3a02436894b42bf68630 Mon Sep 17 00:00:00 2001 From: Resham Joshi <65915470+iamtoruk@users.noreply.github.com> Date: Mon, 12 Oct 2026 15:55:22 -0700 Subject: [PATCH] Windows tray: fall back past a broken managed CLI to Volta and PATH installs The desktop app starts the tray with CODEBURN_BIN set to its own codeburn-cli.cmd launcher, which the tray took as final. When that launcher failed (#1750: exit 1, "The system cannot find the path specified"), the Volta install under a relocated VOLTA_HOME was never tried. The setup check now probes CODEBURN_BIN, every install the PATH and prefix search finds, then the recorded launcher, and keeps the first whose --version answers. When none does, the error lists each path with its own failure. Export and terminal actions use the probed CLI. The desktop app no longer overwrites a CODEBURN_BIN the person set themselves. --- app/electron/menubar.test.ts | 11 ++ app/electron/menubar.ts | 5 +- windows/src-tauri/src/cli.rs | 228 ++++++++++++++++++++++++++++++----- windows/src-tauri/src/lib.rs | 11 +- windows/src/styles.css | 2 +- 5 files changed, 219 insertions(+), 38 deletions(-) diff --git a/app/electron/menubar.test.ts b/app/electron/menubar.test.ts index 7b72c5abe..ed53f19b9 100644 --- a/app/electron/menubar.test.ts +++ b/app/electron/menubar.test.ts @@ -1178,6 +1178,17 @@ describe('MenubarCompanion', () => { expect(launchEnvs).toEqual([{ [TRAY_CLI_ENV]: launcher }, { [TRAY_CLI_ENV]: launcher }]) }) + it('leaves a CODEBURN_BIN of the person\'s own in charge', async () => { + stageMsi() + + await new MenubarCompanion(withCli({ + env: { SystemRoot: 'C:\\Windows', LOCALAPPDATA: join(sandbox, 'Local'), CODEBURN_BUNDLED_CLI: entry, [TRAY_CLI_ENV]: 'D:\\Data\\Volta\\bin\\codeburn.cmd' }, + })).bootstrap() + + expect(launches).toHaveLength(1) + expect(launchEnvs).toEqual([undefined]) + }) + it('leaves the switch off out of it: a login start needs the launcher either way', async () => { stageMsi() writeCompanionSettings(stateDir, { ...DEFAULT_COMPANION_SETTINGS, menuBar: false, sidebar: false, trayExePath: null, seeded: true }) diff --git a/app/electron/menubar.ts b/app/electron/menubar.ts index 8bb40c1c3..1bc08b9c5 100644 --- a/app/electron/menubar.ts +++ b/app/electron/menubar.ts @@ -567,10 +567,11 @@ export class MenubarCompanion { /** * What a tray app started from here inherits. The recorded path in the settings file is * what an autostarted tray app reads, and it is read after PATH; this makes the very first - * launch work too, before the tray app has read anything at all. + * launch work too, before the tray app has read anything at all. A CODEBURN_BIN of the + * person's own is left to reach the tray app as it is. */ private launchEnv(): NodeJS.ProcessEnv | undefined { - return this.cliLauncher ? { [TRAY_CLI_ENV]: this.cliLauncher } : undefined + return this.cliLauncher && !this.deps.env[TRAY_CLI_ENV] ? { [TRAY_CLI_ENV]: this.cliLauncher } : undefined } /** diff --git a/windows/src-tauri/src/cli.rs b/windows/src-tauri/src/cli.rs index 3794f7a57..2f6d5f7b3 100644 --- a/windows/src-tauri/src/cli.rs +++ b/windows/src-tauri/src/cli.rs @@ -162,41 +162,61 @@ pub enum DockQuota { } impl CodeburnCli { - /// Honours `CODEBURN_BIN` only when every whitespace-delimited token passes the - /// allowlist. Otherwise resolves `codeburn` from PATH and the usual npm locations. + /// The first of [`CodeburnCli::candidates`], or the bare name when there are none. What + /// the setup check settles on is [`CodeburnCli::probe`], which also knows whether it runs. pub fn resolve() -> Self { + Self::candidates() + .into_iter() + .next() + .unwrap_or_else(|| Self::at(default_program_name())) + } + + /// `CODEBURN_BIN` first, then PATH and the usual install prefixes, then the CLI the + /// CodeBurn desktop app leaves behind, so a global `codeburn` still wins wherever there + /// is one. + fn candidates() -> Vec { + preferred_order(Self::from_env(), locate_cli(), recorded_desktop_cli()) + } + + /// Runs `--version` on each candidate in turn and settles on the first that answers. One + /// that is there but will not start (a launcher pointing at an executable that moved, a + /// shim whose node is gone) is passed over, so it cannot hide a working install behind it. + pub async fn probe() -> (Self, CliStatus) { + let mut candidates = Self::candidates(); + if candidates.is_empty() { + candidates.push(Self::at(default_program_name())); + } + first_answering(candidates).await + } + + /// Honours `CODEBURN_BIN` only when every whitespace-delimited token passes the + /// allowlist. + fn from_env() -> Option { let raw = env::var("CODEBURN_BIN").unwrap_or_default(); if raw.is_empty() { - return Self::default_program(); + return None; } // A bare path (which may contain spaces, e.g. under Program Files) is used whole; // only otherwise is the value split into program + leading arguments. if is_safe_arg(&raw) && std::path::Path::new(&raw).is_file() { - return CodeburnCli { - program: raw, - extra_args: vec![], - }; + return Some(Self::at(raw)); } let parts: Vec = raw.split_whitespace().map(String::from).collect(); if parts.iter().all(|p| is_safe_arg(p)) { if let Some((first, rest)) = parts.split_first() { - return CodeburnCli { + return Some(CodeburnCli { program: first.clone(), extra_args: rest.to_vec(), - }; + }); } } crate::log_line!("codeburn-menubar: refusing unsafe CODEBURN_BIN; falling back to `codeburn`"); - Self::default_program() + None } - /// PATH and the usual install prefixes first, then the CLI the CodeBurn desktop app - /// leaves behind, so a global `codeburn` still wins wherever there is one. - fn default_program() -> Self { + fn at(program: String) -> Self { CodeburnCli { - program: locate_cli() - .or_else(recorded_desktop_cli) - .unwrap_or_else(default_program_name), + program, extra_args: vec![], } } @@ -506,6 +526,50 @@ impl CodeburnCli { } } +fn preferred_order( + from_env: Option, + found: Vec, + recorded: Option, +) -> Vec { + let mut out: Vec = Vec::new(); + let rest = found.into_iter().chain(recorded).map(CodeburnCli::at); + for cli in from_env.into_iter().chain(rest) { + if !out.iter().any(|seen| seen.program == cli.program) { + out.push(cli); + } + } + out +} + +/// Expects at least one candidate. When none answers, the status names every program tried +/// and what each said, rather than only the first. +async fn first_answering(candidates: Vec) -> (CodeburnCli, CliStatus) { + let mut failed: Vec<(CodeburnCli, CliStatus)> = Vec::new(); + for cli in candidates { + let status = cli.status().await; + if status.found { + return (cli, status); + } + failed.push((cli, status)); + } + let mut status = failed[0].1.clone(); + if failed.len() > 1 { + status.program = failed + .iter() + .map(|(cli, _)| cli.program.as_str()) + .collect::>() + .join(", "); + status.error = Some( + failed + .iter() + .map(|(cli, s)| format!("{}: {}", cli.program, s.error.as_deref().unwrap_or(""))) + .collect::>() + .join("\n"), + ); + } + (failed.swap_remove(0).0, status) +} + /// Reads until the pipe closes, capping what is kept and touching the activity marker on /// every chunk. Reading in chunks rather than to the end is what makes the marker possible. async fn drain( @@ -671,14 +735,14 @@ pub fn parse_version(text: &str) -> Option<(u32, u32, u32)> { /// launched from Explorer or at login, before (or long after) `npm install -g codeburn` /// changed the user's PATH, so we also read the live PATH from the registry on Windows and /// probe the standard npm / node install prefixes. -fn locate_cli() -> Option { +fn locate_cli() -> Vec { find_in_search_dirs(&candidate_names()) } /// Same search for Claude Code's own binary, so "Connect Claude" spawns an absolute path /// instead of letting the console shell resolve a bare `claude`. fn locate_claude() -> Option { - find_in_search_dirs(&CLAUDE_NAMES) + find_in_search_dirs(&CLAUDE_NAMES).into_iter().next() } /// The key the CodeBurn desktop app writes into `windows-settings.json` (the same file this @@ -718,7 +782,7 @@ fn recorded_cli_candidate(settings: &serde_json::Map) -> Option Option { +fn find_in_search_dirs(names: &[&str]) -> Vec { let mut dirs: Vec = Vec::new(); if let Some(path) = env::var_os("PATH") { dirs.extend(env::split_paths(&path)); @@ -732,16 +796,20 @@ fn find_in_search_dirs(names: &[&str]) -> Option { /// and the registry PATH can hold relative entries too; `PathBuf::from("").join("codeburn.cmd")` /// resolves against the current directory, which for a tray app launched at login is /// whatever Explorer handed it. A binary planted there must never win. -fn find_in_dirs(dirs: &[PathBuf], names: &[&str]) -> Option { +fn find_in_dirs(dirs: &[PathBuf], names: &[&str]) -> Vec { + let mut out: Vec = Vec::new(); for dir in dirs.iter().filter(|d| d.is_absolute()) { for name in names { let candidate = dir.join(name); if candidate.is_file() { - return Some(candidate.to_string_lossy().into_owned()); + let path = candidate.to_string_lossy().into_owned(); + if !out.contains(&path) { + out.push(path); + } } } } - None + out } fn candidate_names() -> Vec<&'static str> { @@ -957,7 +1025,13 @@ pub fn is_allowed_subcommand(args: &[&str]) -> bool { /// detached headless spawn. Windows: opens a console via `cmd /C start`. Never /// interpolates through a shell -- argv throughout. pub fn spawn_in_terminal(app: &AppHandle, subcommand: &[&str]) -> Result<()> { - let cli = CodeburnCli::resolve(); + use tauri::Manager; + let cli = app + .state::() + .cli + .lock() + .map_err(|e| anyhow!("{e}"))? + .clone(); spawn_program_in_terminal(app, &cli, subcommand) } @@ -1026,7 +1100,7 @@ fn spawn_program_in_terminal(_app: &AppHandle, cli: &CodeburnCli, subcommand: &[ // Only the unresolved default name is worth a second lookup; anything else is // either already absolute or a CODEBURN_BIN the user chose. let program = if cli.program == default_program_name() { - locate_cli().unwrap_or_else(|| cli.program.clone()) + locate_cli().into_iter().next().unwrap_or_else(|| cli.program.clone()) } else { cli.program.clone() }; @@ -1218,7 +1292,7 @@ mod which { pub fn which(program: &str) -> Result { let path = env::var_os("PATH").ok_or(())?; let dirs: Vec = env::split_paths(&path).collect(); - super::find_in_dirs(&dirs, &[program]).map(PathBuf::from).ok_or(()) + super::find_in_dirs(&dirs, &[program]).into_iter().next().map(PathBuf::from).ok_or(()) } } @@ -1269,16 +1343,17 @@ mod tests { // Empty and relative entries are ignored even though the file is reachable // through them once the process CWD is the temp dir. let unsafe_dirs = vec![PathBuf::from(""), PathBuf::from("."), PathBuf::from("..")]; - assert_eq!(find_in_dirs(&unsafe_dirs, &[name]), None); + assert!(find_in_dirs(&unsafe_dirs, &[name]).is_empty()); - // The same name behind an absolute entry is found. - let found = find_in_dirs(std::slice::from_ref(&dir), &[name]).expect("absolute entry should match"); - assert!(PathBuf::from(&found).is_absolute()); - assert!(found.ends_with(name)); + // The same name behind an absolute entry is found, once however often the entry repeats. + let found = find_in_dirs(&[dir.clone(), dir.clone()], &[name]); + assert_eq!(found.len(), 1); + assert!(PathBuf::from(&found[0]).is_absolute()); + assert!(found[0].ends_with(name)); // An unsafe entry ahead of a good one cannot shadow it. let mixed = vec![PathBuf::from(""), dir.clone()]; - assert_eq!(find_in_dirs(&mixed, &[name]), Some(found)); + assert_eq!(find_in_dirs(&mixed, &[name]), found); std::fs::remove_file(&planted).ok(); } @@ -1312,6 +1387,97 @@ mod tests { assert_eq!(recorded_cli_candidate(&serde_json::Map::new()), None); } + /// CODEBURN_BIN, then every install the search turned up (a relocated Volta home among + /// them), then the desktop app's launcher; a path reached twice is tried once. + #[test] + fn candidates_keep_the_override_first_and_the_desktop_launcher_last() { + let launcher = "/local/codeburn-menubar/codeburn-cli.cmd".to_string(); + let volta = "/data/volta/bin/codeburn.cmd".to_string(); + let npm = "/roaming/npm/codeburn.cmd".to_string(); + let programs = |list: Vec| list.into_iter().map(|c| c.program).collect::>(); + + let ordered = preferred_order( + Some(CodeburnCli::at(launcher.clone())), + vec![volta.clone(), npm.clone(), volta.clone()], + Some(launcher.clone()), + ); + assert_eq!(programs(ordered), vec![launcher.clone(), volta.clone(), npm.clone()]); + + let ordered = preferred_order(None, vec![volta.clone()], Some(launcher.clone())); + assert_eq!(programs(ordered), vec![volta, launcher]); + assert!(preferred_order(None, vec![], None).is_empty()); + } + + #[cfg(unix)] + fn script(dir: &std::path::Path, name: &str, body: &str) -> String { + use std::os::unix::fs::PermissionsExt; + let path = dir.join(name); + std::fs::write(&path, format!("#!/bin/sh\n{body}\n")).unwrap(); + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o755)).unwrap(); + path.to_string_lossy().into_owned() + } + + #[cfg(unix)] + fn scratch(name: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!("codeburn-probe-{name}-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + dir + } + + /// #1750: the desktop app's launcher was handed over as CODEBURN_BIN and exited 1 ("The + /// system cannot find the path specified"), and the Volta install behind it was never + /// tried. A launcher that is gone, or one that runs and fails, now gives way. + #[cfg(unix)] + #[tokio::test] + async fn a_missing_or_failing_launcher_gives_way_to_the_next_install() { + let dir = scratch("fallback"); + let broken = script(&dir, "codeburn-cli.cmd", "echo 'The system cannot find the path specified.' >&2; exit 1"); + let missing = dir.join("gone").join("codeburn-cli.cmd").to_string_lossy().into_owned(); + let volta = script(&dir, "codeburn", "echo 0.9.27"); + + let (cli, status) = first_answering(vec![ + CodeburnCli::at(missing), + CodeburnCli::at(broken), + CodeburnCli::at(volta.clone()), + ]) + .await; + assert_eq!(cli.program, volta); + assert!(status.found && status.compatible, "{status:?}"); + assert_eq!(status.program, volta); + assert_eq!(status.version.as_deref(), Some("0.9.27")); + + std::fs::remove_dir_all(&dir).ok(); + } + + #[cfg(unix)] + #[tokio::test] + async fn when_nothing_answers_the_error_names_every_path_tried() { + let dir = scratch("all-fail"); + let broken = script(&dir, "codeburn-cli.cmd", "echo 'path not found' >&2; exit 1"); + let missing = dir.join("gone").join("codeburn").to_string_lossy().into_owned(); + + let (cli, status) = first_answering(vec![ + CodeburnCli::at(broken.clone()), + CodeburnCli::at(missing.clone()), + ]) + .await; + assert_eq!(cli.program, broken); + assert!(!status.found); + assert_eq!(status.program, format!("{broken}, {missing}")); + let error = status.error.unwrap(); + let lines: Vec<&str> = error.lines().collect(); + assert_eq!(lines.len(), 2, "{error}"); + assert!(lines[0].starts_with(&broken) && lines[0].contains("path not found"), "{error}"); + assert!(lines[1].starts_with(&missing), "{error}"); + + // A single candidate reads exactly as it did before there was a list. + let (_, alone) = first_answering(vec![CodeburnCli::at(broken.clone())]).await; + assert_eq!(alone.program, broken); + assert!(!alone.error.unwrap().starts_with(&broken)); + + std::fs::remove_dir_all(&dir).ok(); + } + /// Nothing resolved at all is a different message from a CLI that is there and would not /// start, because the two have different answers. #[test] diff --git a/windows/src-tauri/src/lib.rs b/windows/src-tauri/src/lib.rs index 1b45b2c80..47e53624e 100644 --- a/windows/src-tauri/src/lib.rs +++ b/windows/src-tauri/src/lib.rs @@ -1024,8 +1024,7 @@ mod commands { /// without restarting the tray app. #[tauri::command] pub async fn cli_status(state: State<'_, AppState>) -> Result { - let fresh = crate::cli::CodeburnCli::resolve(); - let status = fresh.status().await; + let (fresh, status) = crate::cli::CodeburnCli::probe().await; if status.found { if let Ok(mut guard) = state.cli.lock() { *guard = fresh; @@ -1117,7 +1116,11 @@ mod commands { /// shell, but it does become a directory name, so it is checked here rather than trusted: /// digits and hyphens only, and the length the format produces. #[tauri::command] - pub async fn export_usage(format: String, stamp: String) -> Result { + pub async fn export_usage( + state: State<'_, AppState>, + format: String, + stamp: String, + ) -> Result { if !matches!(format.as_str(), "csv" | "json") { return Err("unknown export format".into()); } @@ -1134,7 +1137,7 @@ mod commands { _ => format!("codeburn-{stamp}"), }; let output = downloads.join(name); - let cli = crate::cli::CodeburnCli::resolve(); + let cli = state.cli.lock().map_err(|e| e.to_string())?.clone(); cli.export_to(&format, &output) .await .map_err(|e| e.to_string())?; diff --git a/windows/src/styles.css b/windows/src/styles.css index 8d46ce0df..1aa2300ba 100644 --- a/windows/src/styles.css +++ b/windows/src/styles.css @@ -1740,7 +1740,7 @@ code { font-family: var(--font-mono); } .setup-details { font-size: 10.5px; color: var(--label-2); max-width: 300px; } .setup-details summary { display: inline-flex; align-items: center; gap: 4px; list-style: none; color: var(--label-3); } .setup-details summary::-webkit-details-marker { display: none; } -.setup-error { margin-top: 6px; text-align: left; font-family: var(--font-mono); font-size: 10px; word-break: break-word; user-select: text; } +.setup-error { margin-top: 6px; text-align: left; font-family: var(--font-mono); font-size: 10px; word-break: break-word; white-space: pre-line; user-select: text; } .setup-error-muted { margin-top: 4px; text-align: left; font-size: 10px; color: var(--label-3); word-break: break-all; } .toggle {