diff --git a/Cargo.lock b/Cargo.lock index 937ead564a..23690e9e71 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -862,6 +862,7 @@ dependencies = [ "async-trait", "axum", "base64 0.22.1", + "dirs", "getrandom 0.4.3", "hex", "nix 0.31.3", diff --git a/crates/buzz-agent/Cargo.toml b/crates/buzz-agent/Cargo.toml index 7889ad34a7..52ede9c18e 100644 --- a/crates/buzz-agent/Cargo.toml +++ b/crates/buzz-agent/Cargo.toml @@ -32,6 +32,7 @@ serde_yaml = { workspace = true } reqwest = { workspace = true, features = ["json", "rustls", "form"] } rmcp = { version = "1", default-features = false, features = ["client", "transport-child-process"] } arc-swap = "1" +dirs = "6" getrandom = "0.4" tracing = { workspace = true } tracing-subscriber = { workspace = true } diff --git a/crates/buzz-agent/src/auth.rs b/crates/buzz-agent/src/auth.rs index 34974fbf0b..df135ad960 100644 --- a/crates/buzz-agent/src/auth.rs +++ b/crates/buzz-agent/src/auth.rs @@ -454,10 +454,21 @@ fn cache_path_for(cfg: &PkceOAuthConfig) -> Result { let dir = match &cfg.cache_dir_override { Some(p) => p.join(&cfg.cache_namespace), None => { - let home = std::env::var("HOME") - .map_err(|_| AgentError::Llm("oauth cache: $HOME not set".into()))?; - PathBuf::from(home) - .join(".config") + // Reading `$HOME` directly made this a hard error on Windows, where + // that variable is normally undefined. The `.config` segment is kept + // on every platform on purpose: switching Windows to `%APPDATA%` (or + // macOS to `dirs::config_dir`) would relocate caches that already + // exist, which is a separate call from fixing the failure. + // + // `$HOME` still wins where it is set — `dirs::home_dir` ignores it on + // Windows, which would move an existing Git Bash cache and defeat the + // per-test `HOME` isolation this module documents above. + let home = std::env::var_os("HOME") + .filter(|value| !value.is_empty()) + .map(std::path::PathBuf::from) + .or_else(dirs::home_dir) + .ok_or_else(|| AgentError::Llm("oauth cache: no home directory".into()))?; + home.join(".config") .join("buzz-agent") .join("oauth") .join(&cfg.cache_namespace) @@ -682,19 +693,42 @@ mod tests { assert!(is_expired(&t)); } - #[test] - fn cache_path_includes_namespace_and_hash() { - // HOME is required; cargo test runs set it. - let cfg = PkceOAuthConfig { + fn demo_oauth_config() -> PkceOAuthConfig { + PkceOAuthConfig { discovery_url: "https://example.com/.well-known".into(), client_id: "abc".into(), scopes: vec!["a".into(), "b".into()], cache_namespace: "demo".into(), cache_dir_override: None, - }; - let p = cache_path_for(&cfg).unwrap(); - assert!(p.to_string_lossy().contains("/buzz-agent/oauth/demo/")); - assert!(p.extension().and_then(|s| s.to_str()) == Some("json")); + } + } + + #[test] + fn cache_path_includes_namespace_and_hash() { + let p = cache_path_for(&demo_oauth_config()).unwrap(); + // Compared component-wise: a literal "/buzz-agent/oauth/demo/" can never + // match on Windows, where the separator is '\'. + let parts: Vec = p + .components() + .map(|c| c.as_os_str().to_string_lossy().into_owned()) + .collect(); + assert!( + parts + .windows(3) + .any(|w| w[0] == "buzz-agent" && w[1] == "oauth" && w[2] == "demo"), + "expected buzz-agent/oauth/demo inside {p:?}" + ); + assert_eq!(p.extension().and_then(|s| s.to_str()), Some("json")); + } + + #[test] + fn cache_path_is_rooted_at_the_platform_home() { + // Regression guard: this read `$HOME` directly and returned Err on + // Windows, where that variable is normally undefined — every OAuth + // provider failed there with "oauth cache: $HOME not set". + let p = cache_path_for(&demo_oauth_config()).unwrap(); + let home = dirs::home_dir().expect("platform must report a home directory"); + assert!(p.starts_with(&home), "{p:?} must sit under {home:?}"); } #[test] diff --git a/crates/buzz-agent/src/hints.rs b/crates/buzz-agent/src/hints.rs index 9fb99f0ebc..48e894c08a 100644 --- a/crates/buzz-agent/src/hints.rs +++ b/crates/buzz-agent/src/hints.rs @@ -7,8 +7,22 @@ const MAX_HINTS_BYTES: usize = 128 * 1024; pub const MAX_SKILL_BODY_BYTES: usize = 32 * 1024; const SKILL_DIRS: &[&str] = &[".agents/skills", ".goose/skills", ".claude/skills"]; +/// `$HOME` is not defined on Windows — it exists only inside shells like Git Bash +/// that export it, and the desktop spawns agents from a GUI process that has no +/// such variable. Reading it directly meant `~/.agents/skills` and friends were +/// never discovered there. `dirs::home_dir` consults the platform's own notion of +/// the profile directory. +/// +/// `$HOME` still wins when it is set. `dirs::home_dir` ignores it on Windows in +/// favour of `USERPROFILE`, which would silently override a deliberate setting — +/// Git Bash sessions export their own `HOME`, and callers that point `HOME` at a +/// scratch directory expect to be obeyed on every platform. Falling back rather +/// than replacing keeps Unix behaviour byte-identical. fn home_dir() -> Option { - std::env::var("HOME").ok().map(PathBuf::from) + if let Some(home) = std::env::var_os("HOME").filter(|value| !value.is_empty()) { + return Some(PathBuf::from(home)); + } + dirs::home_dir() } #[derive(Clone)] @@ -616,6 +630,13 @@ mod tests { assert_eq!(skills[0].name, "local"); } + #[test] + fn home_dir_resolves_on_this_platform() { + // Regression guard: reading `$HOME` yielded None on Windows, so the + // global skill directories were silently never scanned there. + assert!(home_dir().is_some()); + } + #[test] fn collect_supporting_files_finds_non_skill_md_files() { let tmp = TempDir::new().unwrap(); diff --git a/desktop/src-tauri/Cargo.lock b/desktop/src-tauri/Cargo.lock index fbaa547a03..54eb3627e5 100644 --- a/desktop/src-tauri/Cargo.lock +++ b/desktop/src-tauri/Cargo.lock @@ -1021,6 +1021,7 @@ dependencies = [ "async-trait", "axum", "base64 0.22.1", + "dirs", "getrandom 0.4.3", "hex", "nix 0.31.3",