From be13b4bb9ce228b21fa3682ce75d75cba5950561 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Mon, 27 Jul 2026 21:30:21 -0400 Subject: [PATCH] fix(desktop): probe legacy Goose install dir on Windows (#3248) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Goose's pre-[#2680](https://github.com/block/buzz/pull/2680) Windows installer unpacked the CLI to `%USERPROFILE%\goose\goose.exe`. That directory is on no standard `PATH`, and `common_binary_paths()` never probed it, so users who installed Goose with the legacy installer stayed permanently undiscovered — the residual half of #2239. `resolve_command_uncached` finds binaries outside `PATH` only by scanning `common_binary_paths()`, so adding the directory there is the whole fix: Windows basename expansion already supplies `goose.exe`/`.cmd`/`.bat`, and discovery, readiness probes, and spawn all route through the same shared resolver. No Goose-specific resolution path is introduced. The entry sits beside the existing Codex `%LOCALAPPDATA%\Programs\OpenAI\Codex\bin` probe in the same `#[cfg(windows)]` block. The regression test is `#[cfg(windows)]` and is CI-reachable, not dead code — the `desktop-build-windows` job runs `cargo test --manifest-path desktop/src-tauri/Cargo.toml --target $env:TARGET` on `windows-latest`. It asserts the probe list rather than planting a binary: `common_binary_paths` is a process-lifetime `OnceLock`, so a test cannot deterministically re-seed `USERPROFILE`, and planting an executable under the real user profile is not an acceptable side effect. Verified locally by widening the `cfg` to build on macOS — the test passes with the probe and fails without it. The `check-file-sizes.mjs` override for `managed_agents/discovery.rs` moves 1835 → 1841, the exact post-`cargo fmt` gate count. Verified both directions: 1841 passes, 1840 fails. Signed-off-by: Will Pfleger --- desktop/scripts/check-file-sizes.mjs | 4 ++- desktop/src-tauri/Cargo.lock | 1 + .../src-tauri/src/managed_agents/discovery.rs | 6 +++++ .../tests/managed_path_resolution.rs | 26 +++++++++++++++++++ 4 files changed, 36 insertions(+), 1 deletion(-) diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index 01605e3c6..6e44481d5 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -344,7 +344,9 @@ const overrides = new Map([ // absent, so AdapterMissing replaces the misleading NotInstalled. Includes // the deliberate-divergence doc comments; net after the inline preset // entries.push block collapsed into the helper. - ["src-tauri/src/managed_agents/discovery.rs", 1835], + // +6: legacy Goose Windows install dir (%USERPROFILE%\goose) probed in + // common_binary_paths so pre-#2680 standalone installs are discoverable. + ["src-tauri/src/managed_agents/discovery.rs", 1841], // BYOH — save_custom_harness_to_dir (backup-swap atomic write) + save_and_warm / // delete_and_warm (persist-mutex serialization for concurrent-safe registry // refresh, B-6). Also: id/collision/load/registry tests (from the file base) + diff --git a/desktop/src-tauri/Cargo.lock b/desktop/src-tauri/Cargo.lock index 7dcf615c0..074b8f739 100644 --- a/desktop/src-tauri/Cargo.lock +++ b/desktop/src-tauri/Cargo.lock @@ -991,6 +991,7 @@ dependencies = [ name = "buzz-core" version = "0.1.0" dependencies = [ + "base64 0.22.1", "chrono", "hex", "hmac 0.13.0", diff --git a/desktop/src-tauri/src/managed_agents/discovery.rs b/desktop/src-tauri/src/managed_agents/discovery.rs index 97498c8c3..71e689330 100644 --- a/desktop/src-tauri/src/managed_agents/discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery.rs @@ -58,6 +58,12 @@ fn common_binary_paths() -> &'static [PathBuf] { .join("bin"), ); } + // Goose's legacy Windows installer (superseded by #2680) unpacked + // to %USERPROFILE%\goose\goose.exe, which is on no standard PATH — + // without this probe those installs stay permanently undiscovered. + if let Some(profile) = std::env::var_os("USERPROFILE") { + paths.push(PathBuf::from(profile).join("goose")); + } } paths }) diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests/managed_path_resolution.rs b/desktop/src-tauri/src/managed_agents/discovery/tests/managed_path_resolution.rs index 2f6b038de..0795bb234 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests/managed_path_resolution.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests/managed_path_resolution.rs @@ -1,5 +1,31 @@ use crate::managed_agents::discovery::{clear_resolve_cache, resolve_command}; +/// The legacy Goose Windows installer wrote `%USERPROFILE%\goose\goose.exe`, +/// a directory on no standard PATH. `resolve_command_uncached` finds binaries +/// outside PATH only by scanning `common_binary_paths()`, so that directory +/// must appear there or those installs stay undiscovered (#2239 residual). +/// +/// Asserts the probe list rather than a planted binary: `common_binary_paths` +/// is a process-lifetime `OnceLock`, so a test cannot re-seed `USERPROFILE` +/// deterministically, and planting an executable under the real user profile +/// is not an acceptable test side effect. +#[cfg(windows)] +#[test] +fn common_binary_paths_probes_legacy_goose_install_dir() { + use std::path::PathBuf; + + let profile = std::env::var_os("USERPROFILE").expect("USERPROFILE is always set on Windows"); + let legacy_dir = PathBuf::from(profile).join("goose"); + + let probed = super::super::common_binary_paths(); + + assert!( + probed.contains(&legacy_dir), + "legacy Goose install dir {} must be probed, got: {probed:?}", + legacy_dir.display() + ); +} + #[cfg(unix)] #[test] fn resolve_command_prefers_buzz_managed_npm_shim_over_path() {