diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 489ba22d2..ebae8f09b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -568,6 +568,56 @@ jobs: -p git-credential-nostr \ -p git-sign-nostr + windows-rust: + name: Windows Rust (x86_64-pc-windows-msvc) + runs-on: windows-latest + # Windows runners are slow and this compiles the workspace + Tauri crate + # cold across four steps; budget generously. + timeout-minutes: 45 + needs: [changes] + if: github.event_name == 'push' || needs.changes.outputs.rust == 'true' || needs.changes.outputs.desktop-rust == 'true' + permissions: + contents: read + env: + TARGET: x86_64-pc-windows-msvc + steps: + - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + # MSVC needs windows.h (aws-lc-sys et al.), so this runs on a real Windows + # runner — hermit, used by the Linux jobs, does not provide MSVC. The + # toolchain (1.95.0 + clippy via profile = default) comes from the + # repo-root rust-toolchain.toml, which the runner's preinstalled rustup + # honors on demand; the host triple already is x86_64-pc-windows-msvc. + - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2 + with: + workspaces: | + . + desktop/src-tauri + key: windows-msvc + save-if: ${{ github.event_name != 'pull_request' }} + # Tauri validates externalBin at compile time, so the Tauri-crate steps + # below fail without these stubs. Mirrors scripts/bundle-sidecars.sh's + # Windows naming (binaries/-.exe); empty files suffice for a + # type-check since nothing executes them. + - name: Create sidecar placeholders + shell: bash + run: | + mkdir -p desktop/src-tauri/binaries + for bin in buzz-acp buzz-agent buzz-dev-mcp git-credential-nostr buzz; do + touch "desktop/src-tauri/binaries/${bin}-${TARGET}.exe" + done + - name: Clippy (workspace) + run: cargo clippy --workspace --all-targets --target $env:TARGET -- -D warnings + - name: Check (workspace) + run: cargo check --workspace --all-targets --target $env:TARGET + - name: Check (Tauri crate) + run: cargo check --manifest-path desktop/src-tauri/Cargo.toml --target $env:TARGET + env: + CMAKE_POLICY_VERSION_MINIMUM: "3.5" + - name: Test (Tauri crate) + run: cargo test --manifest-path desktop/src-tauri/Cargo.toml --target $env:TARGET + env: + CMAKE_POLICY_VERSION_MINIMUM: "3.5" + desktop-build-macos: name: Desktop Build (macOS) runs-on: macos-latest diff --git a/crates/buzz-agent/src/mcp.rs b/crates/buzz-agent/src/mcp.rs index 2c39de270..6a7b61dca 100644 --- a/crates/buzz-agent/src/mcp.rs +++ b/crates/buzz-agent/src/mcp.rs @@ -60,6 +60,13 @@ const PASSTHROUGH_ENV: &[&str] = &[ "BUZZ_RELAY_URL", ]; +// Windows has no $TMPDIR/$HOME. TMP/TEMP/USERPROFILE are what +// std::env::temp_dir() consults — without them it falls back to C:\Windows, +// which child processes can't write to (PermissionDenied). USERPROFILE is the +// always-set floor. LOCALAPPDATA/APPDATA carry child-tool config (git, etc.). +#[cfg(windows)] +const PASSTHROUGH_ENV_WINDOWS: &[&str] = &["TMP", "TEMP", "USERPROFILE", "LOCALAPPDATA", "APPDATA"]; + type Client = RunningService; #[derive(Clone)] @@ -697,6 +704,12 @@ async fn spawn_one( cmd.env(k, v); } } + #[cfg(windows)] + for k in PASSTHROUGH_ENV_WINDOWS { + if let Ok(v) = std::env::var(k) { + cmd.env(k, v); + } + } for (k, v) in &spec.env { cmd.env(k, v); } @@ -966,6 +979,19 @@ mod content_tests { use super::*; use rmcp::model::Content; + #[cfg(windows)] + #[test] + fn windows_passthrough_includes_temp_dir_vars() { + // std::env::temp_dir() consults these in order; without them it falls + // back to C:\Windows, which children can't write to. + for var in ["TMP", "TEMP", "USERPROFILE"] { + assert!( + PASSTHROUGH_ENV_WINDOWS.contains(&var), + "{var} must pass through or temp_dir() falls back to C:\\Windows" + ); + } + } + #[test] fn tool_result_content_preserves_images() { let blocks = vec![ diff --git a/crates/buzz-dev-mcp/src/lib.rs b/crates/buzz-dev-mcp/src/lib.rs index edc9b9fb7..ab4f6ad4c 100644 --- a/crates/buzz-dev-mcp/src/lib.rs +++ b/crates/buzz-dev-mcp/src/lib.rs @@ -137,7 +137,7 @@ impl ServerHandler for DevMcp { pub fn run() -> Result<(), Box> { let argv0 = std::env::args().next().unwrap_or_default(); let cmd = Path::new(&argv0) - .file_name() + .file_stem() .and_then(|n| n.to_str()) .unwrap_or("") .to_ascii_lowercase(); diff --git a/crates/buzz-dev-mcp/src/shim.rs b/crates/buzz-dev-mcp/src/shim.rs index 0ecef5061..36a07a18c 100644 --- a/crates/buzz-dev-mcp/src/shim.rs +++ b/crates/buzz-dev-mcp/src/shim.rs @@ -40,12 +40,13 @@ impl Shim { } let original = std::env::var_os("PATH").unwrap_or_default(); - let mut new_path = std::ffi::OsString::from(dir.path()); - if !original.is_empty() { - new_path.push(":"); - new_path.push(&original); - } - let path_env = new_path.to_string_lossy().into_owned(); + let mut entries = vec![PathBuf::from(dir.path())]; + entries.extend(std::env::split_paths(&original)); + // join_paths uses the platform separator (':' on Unix, ';' on Windows). + let path_env = std::env::join_paths(entries) + .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidInput, e))? + .to_string_lossy() + .into_owned(); // Read and unconditionally remove NOSTR_PRIVATE_KEY from this process's // env. The key must never leak to child processes regardless of whether @@ -234,6 +235,9 @@ fn symlink(src: &Path, dst: &Path) -> std::io::Result<()> { #[cfg(not(unix))] fn symlink(src: &Path, dst: &Path) -> std::io::Result<()> { + // No symlinks without elevation on Windows; copy instead. The target needs + // a .exe extension or PATH lookup (via PATHEXT) won't treat it as runnable. + let dst = dst.with_extension("exe"); std::fs::copy(src, dst).map(|_| ()) } diff --git a/desktop/src-tauri/Cargo.toml b/desktop/src-tauri/Cargo.toml index 548094c94..0c8b22644 100644 --- a/desktop/src-tauri/Cargo.toml +++ b/desktop/src-tauri/Cargo.toml @@ -33,7 +33,7 @@ ctrlc = { version = "3", features = ["termination"] } objc2-app-kit = { version = "0.3.2", default-features = false, features = ["NSHapticFeedback"] } [target.'cfg(windows)'.dependencies] -windows-sys = { version = "0.61", features = ["Win32_Storage_FileSystem"] } +windows-sys = { version = "0.61", features = ["Win32_Storage_FileSystem", "Win32_System_JobObjects", "Win32_System_Threading", "Win32_Foundation"] } [dependencies] atomic-write-file = "0.3" diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index 37bad60f8..dc64dc826 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -12,8 +12,7 @@ use crate::{ sync_managed_agent_processes, try_regenerate_nest, validate_provider_config, BackendKind, BackendProviderInfo, CreateManagedAgentRequest, CreateManagedAgentResponse, ManagedAgentLogResponse, ManagedAgentRecord, ManagedAgentSummary, RelayMeshConfig, - DEFAULT_ACP_COMMAND, DEFAULT_AGENT_COMMAND, DEFAULT_AGENT_PARALLELISM, - DEFAULT_AGENT_TURN_TIMEOUT_SECONDS, + DEFAULT_ACP_COMMAND, DEFAULT_AGENT_PARALLELISM, DEFAULT_AGENT_TURN_TIMEOUT_SECONDS, }, relay::{relay_ws_url_with_override, sync_managed_agent_profile}, util::now_iso, @@ -456,8 +455,8 @@ pub async fn create_managed_agent( .as_deref() .map(str::trim) .filter(|value| !value.is_empty()) - .unwrap_or(DEFAULT_AGENT_COMMAND) - .to_string(); + .map(str::to_string) + .unwrap_or_else(crate::managed_agents::default_agent_command); let agent_args = normalize_agent_args( &agent_command, input diff --git a/desktop/src-tauri/src/managed_agents/discovery.rs b/desktop/src-tauri/src/managed_agents/discovery.rs index 88f5c2d6a..21ca25d58 100644 --- a/desktop/src-tauri/src/managed_agents/discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery.rs @@ -228,6 +228,21 @@ pub(crate) fn known_acp_runtime_exact(id: &str) -> Option<&'static KnownAcpRunti KNOWN_ACP_RUNTIMES.iter().find(|p| p.id == id) } +/// The agent command a freshly-created agent defaults to when the create +/// request supplies none. Resolves the bundled `buzz-agent` from the catalog — +/// the same shape `mesh_llm::preset` uses — so the default can't drift from the +/// provider definition. Falls back to the id if the catalog entry is missing. +/// +/// The previous default was the bare global `goose`, which is not on PATH on a +/// stock Windows install: every worker failed with `program not found`. The +/// bundled `buzz-agent` ships with the app and resolves on every platform. +pub fn default_agent_command() -> String { + known_acp_runtime_exact("buzz-agent") + .and_then(|p| p.commands.first().copied()) + .unwrap_or("buzz-agent") + .to_string() +} + fn default_agent_args(command: &str) -> Option> { match normalize_command_identity(command).as_str() { "goose" => Some(vec!["acp".to_string()]), @@ -562,8 +577,9 @@ mod tests { use std::path::PathBuf; use super::{ - classify_runtime, find_via_login_shell, managed_agent_avatar_url, normalize_agent_args, - BUZZ_AGENT_AVATAR_URL, CLAUDE_CODE_AVATAR_URL, CODEX_AVATAR_URL, GOOSE_AVATAR_URL, + classify_runtime, default_agent_command, find_via_login_shell, managed_agent_avatar_url, + normalize_agent_args, BUZZ_AGENT_AVATAR_URL, CLAUDE_CODE_AVATAR_URL, CODEX_AVATAR_URL, + GOOSE_AVATAR_URL, }; use crate::managed_agents::AcpAvailabilityStatus; @@ -599,6 +615,18 @@ mod tests { assert!(managed_agent_avatar_url("custom-agent").is_none()); } + #[test] + fn default_agent_command_resolves_bundled_buzz_agent() { + // The create-path default must be the bundled buzz-agent, never the + // bare `goose` that isn't on PATH on a stock Windows install. + assert_eq!(default_agent_command(), "buzz-agent"); + // And buzz-agent takes no `acp` arg — confirm no arg leakage from the default. + assert_eq!( + normalize_agent_args(&default_agent_command(), vec!["acp".into()]), + Vec::::new() + ); + } + #[test] fn normalizes_claude_and_codex_args_to_empty() { assert_eq!( diff --git a/desktop/src-tauri/src/managed_agents/mod.rs b/desktop/src-tauri/src/managed_agents/mod.rs index f10376a27..d22598ed8 100644 --- a/desktop/src-tauri/src/managed_agents/mod.rs +++ b/desktop/src-tauri/src/managed_agents/mod.rs @@ -5,6 +5,8 @@ mod nest; mod persona_avatars; mod persona_card; mod personas; +#[cfg(windows)] +mod process_lifecycle; #[cfg(feature = "mesh-llm")] mod relay_mesh; mod restore; @@ -20,6 +22,8 @@ pub use env_vars::*; pub use nest::*; pub use persona_card::*; pub use personas::*; +#[cfg(windows)] +pub use process_lifecycle::*; #[cfg(feature = "mesh-llm")] pub use relay_mesh::*; pub use restore::*; diff --git a/desktop/src-tauri/src/managed_agents/process_lifecycle.rs b/desktop/src-tauri/src/managed_agents/process_lifecycle.rs new file mode 100644 index 000000000..b3972d74e --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/process_lifecycle.rs @@ -0,0 +1,150 @@ +//! Windows process-tree lifecycle primitives for managed agents. +//! +//! The Unix teardown uses `process_group(0)` + group signals (in `runtime.rs`). +//! Windows has no process groups, so the harness's 24 agent workers + MCP +//! servers are reaped two ways here: +//! - [`JobHandle`] / [`create_job_for_child`] — the in-process stop path. A +//! Job Object owns the tree and `JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE` kills +//! it when the handle drops. +//! - [`taskkill_tree`] — the after-restart path, where only the PID survives +//! in the record and no job handle is available. +//! +//! This module is `#[cfg(windows)]`-only; nothing here compiles on other +//! platforms. + +use windows_sys::Win32::Foundation::HANDLE; + +/// Win32 Job Object that owns the harness process and (via Windows' default +/// child-inheritance) every process it spawns. Dropping the handle with +/// `JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE` set kills the whole tree — the Windows +/// mirror of the Unix `process_group(0)` + group-signal teardown. This is what +/// guarantees the 24 agent workers + MCP servers die when we stop or when the +/// app exits, instead of being orphaned by a bare `Child::kill()`. +pub struct JobHandle(HANDLE); + +// The handle is owned exclusively by this wrapper; moving it across threads is +// sound (the spawn path in restore.rs runs in a thread scope). +unsafe impl Send for JobHandle {} + +impl std::fmt::Debug for JobHandle { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("JobHandle(..)") + } +} + +impl Drop for JobHandle { + fn drop(&mut self) { + // KILL_ON_JOB_CLOSE means the tree dies when the LAST handle closes. + // We hold the only handle (not inheritable), so this reaps the tree. + unsafe { windows_sys::Win32::Foundation::CloseHandle(self.0) }; + } +} + +/// Create a Job Object, assign `pid` to it, and configure it to kill the whole +/// tree when the returned handle is dropped. Returns `None` on any failure so +/// the caller can fall back to `Child::kill()` — a degraded teardown beats a +/// failed spawn. +/// +/// Assignment happens immediately after spawn, on the same parent thread. The +/// child (buzz-acp) does spawn its 24 workers before it connects to the relay, +/// so the window between our spawn and our assignment is NOT structurally empty. +/// What closes it is assign-latency: `OpenProcess` + `AssignProcessToJobObject` +/// are a few synchronous Win32 calls (microseconds), while buzz-acp must init +/// tokio, parse its config, and spawn 24 children (tens-to-hundreds of ms), so +/// the assign reliably wins before any worker exists. Once assigned, Windows +/// places every subsequently-spawned descendant in the job automatically. +/// +/// `CREATE_SUSPENDED` -> assign -> `ResumeThread` would make the window airtight +/// regardless of child timing, but it requires raw `CreateProcessW`/`ResumeThread` +/// (materially more unsafe Win32) to close a microsecond race, so it is +/// deliberately not used here. +fn create_job_for_child(pid: u32) -> Option { + use std::ptr::null; + use windows_sys::Win32::Foundation::{CloseHandle, FALSE}; + use windows_sys::Win32::System::JobObjects::{ + AssignProcessToJobObject, CreateJobObjectW, JobObjectExtendedLimitInformation, + SetInformationJobObject, JOBOBJECT_EXTENDED_LIMIT_INFORMATION, + JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE, + }; + use windows_sys::Win32::System::Threading::{ + OpenProcess, PROCESS_SET_QUOTA, PROCESS_TERMINATE, + }; + + unsafe { + let job = CreateJobObjectW(null(), null()); + if job.is_null() { + return None; + } + + let mut info: JOBOBJECT_EXTENDED_LIMIT_INFORMATION = std::mem::zeroed(); + info.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; + let ok = SetInformationJobObject( + job, + JobObjectExtendedLimitInformation, + &info as *const _ as *const _, + std::mem::size_of::() as u32, + ); + if ok == FALSE { + CloseHandle(job); + return None; + } + + let process = OpenProcess(PROCESS_SET_QUOTA | PROCESS_TERMINATE, FALSE, pid); + if process.is_null() { + CloseHandle(job); + return None; + } + let assigned = AssignProcessToJobObject(job, process); + CloseHandle(process); + if assigned == FALSE { + CloseHandle(job); + return None; + } + + Some(JobHandle(job)) + } +} + +/// Kill the entire process tree rooted at `pid` via `taskkill /T`, the closest +/// equivalent to the Unix process-group kill. Used on the after-restart path +/// where no job handle survived. `CREATE_NO_WINDOW` keeps taskkill's own +/// console from flashing. +pub fn taskkill_tree(pid: u32) -> Result<(), String> { + use std::os::windows::process::CommandExt; + const CREATE_NO_WINDOW: u32 = 0x0800_0000; + let status = std::process::Command::new("taskkill") + .args(["/T", "/F", "/PID", &pid.to_string()]) + .creation_flags(CREATE_NO_WINDOW) + .status() + .map_err(|error| format!("failed to run taskkill for pid {pid}: {error}"))?; + if status.success() { + Ok(()) + } else { + Err(format!( + "taskkill exited with status {status} for pid {pid}" + )) + } +} + +/// Assign a freshly-spawned harness `child` to a Job Object and package it into +/// a [`ManagedAgentProcess`]. On job-assignment failure the process is still +/// returned with `job: None` — teardown then falls back to `Child::kill()`, +/// which kills only the harness (a degraded teardown beats a failed spawn). +pub fn finish_spawn( + child: std::process::Child, + log_path: std::path::PathBuf, + agent_name: &str, +) -> super::ManagedAgentProcess { + let job = create_job_for_child(child.id()); + if job.is_none() { + eprintln!( + "buzz-desktop: failed to assign agent {agent_name} to a Job Object; \ + teardown will fall back to killing only the harness process" + ); + } + super::ManagedAgentProcess { + child, + log_path, + job, + } +} diff --git a/desktop/src-tauri/src/managed_agents/restore.rs b/desktop/src-tauri/src/managed_agents/restore.rs index 2bad0fd36..3469dc80d 100644 --- a/desktop/src-tauri/src/managed_agents/restore.rs +++ b/desktop/src-tauri/src/managed_agents/restore.rs @@ -9,7 +9,7 @@ use crate::util; use std::sync::atomic::{AtomicBool, Ordering}; use tauri::Manager; -type SpawnResult = Result<(std::process::Child, std::path::PathBuf), String>; +type SpawnResult = Result; type AgentSpawnResult = (String, SpawnResult); /// Restore managed agents that were running before the app was closed. @@ -176,15 +176,15 @@ pub async fn restore_managed_agents_on_launch( Err(_) => continue, }; match result { - Ok((child, log_path)) => { + Ok(process) => { let now = util::now_iso(); record.updated_at = now.clone(); - record.runtime_pid = Some(child.id()); + record.runtime_pid = Some(process.child.id()); record.last_started_at = Some(now); record.last_stopped_at = None; record.last_exit_code = None; record.last_error = None; - runtimes.insert(pubkey.clone(), ManagedAgentProcess { child, log_path }); + runtimes.insert(pubkey.clone(), process); successfully_spawned.push(pubkey); } Err(error) => { diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index d412e790e..328b888fd 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -284,9 +284,17 @@ pub(crate) fn terminate_process(pid: u32) -> Result<(), String> { Ok(()) } -#[cfg(not(unix))] +#[cfg(windows)] +pub(crate) fn terminate_process(pid: u32) -> Result<(), String> { + // No job handle is available on this path (e.g. after an app restart, when + // we only recovered the PID from the record), so fall back to taskkill on + // the whole tree. + super::process_lifecycle::taskkill_tree(pid) +} + +#[cfg(not(any(unix, windows)))] pub(crate) fn terminate_process(_pid: u32) -> Result<(), String> { - Err("managed agent shutdown after app restart is only supported on Unix".to_string()) + Err("managed agent shutdown after app restart is not supported on this platform".to_string()) } /// Send SIGTERM to all given PIDs (as process groups), wait, then SIGKILL @@ -1473,7 +1481,7 @@ pub fn spawn_agent_child( app: &AppHandle, record: &ManagedAgentRecord, owner_hex: Option<&str>, -) -> Result<(std::process::Child, std::path::PathBuf), String> { +) -> Result { let log_path = managed_agent_log_path(app, &record.pubkey)?; append_log_marker( &log_path, @@ -1516,22 +1524,25 @@ pub fn spawn_agent_child( // - bundled sidecars (buzz, buzz-acp, etc.) via exe parent (Contents/MacOS/) // - runtimes (node, python, etc.) via login shell PATH let augmented_path = { - let mut parts: Vec = Vec::new(); + let mut parts: Vec = Vec::new(); if let Some(home) = dirs::home_dir() { - parts.push(home.join(".local").join("bin").display().to_string()); + parts.push(home.join(".local").join("bin")); } if let Ok(exe) = std::env::current_exe() { if let Some(parent) = exe.parent() { - parts.push(parent.display().to_string()); + parts.push(parent.to_path_buf()); } } if let Some(shell_path) = login_shell_path() { - parts.push(shell_path); + parts.push(std::path::PathBuf::from(shell_path)); } if parts.is_empty() { None } else { - Some(parts.join(":")) + // join_paths uses the platform separator (':' on Unix, ';' on Windows). + std::env::join_paths(parts) + .ok() + .map(|s| s.to_string_lossy().into_owned()) } }; @@ -1730,6 +1741,15 @@ pub fn spawn_agent_child( use std::os::unix::process::CommandExt; command.process_group(0); } + // Windows: suppress the harness console window. Without this a bare + // terminal pops for buzz-acp.exe and lingers (the app itself sets + // windows_subsystem="windows", but the spawned child does not inherit it). + #[cfg(windows)] + { + use std::os::windows::process::CommandExt; + const CREATE_NO_WINDOW: u32 = 0x0800_0000; + command.creation_flags(CREATE_NO_WINDOW); + } let child = command.spawn().map_err(|error| { format!( @@ -1741,7 +1761,16 @@ pub fn spawn_agent_child( let _ = super::write_agent_pid_file(app, &record.pubkey, child.id()); - Ok((child, log_path)) + // Windows: assign the harness to a Job Object so its whole tree dies with + // the handle. The Unix process-group equivalent is set above. + #[cfg(windows)] + return Ok(super::process_lifecycle::finish_spawn( + child, + log_path, + &record.name, + )); + #[cfg(not(windows))] + Ok(crate::managed_agents::ManagedAgentProcess { child, log_path }) } fn child_rust_log_filter() -> String { @@ -1798,20 +1827,17 @@ pub fn start_managed_agent_process( record.runtime_pid = None; } - let (child, log_path) = spawn_agent_child(app, record, owner_hex)?; + let process = spawn_agent_child(app, record, owner_hex)?; let now = now_iso(); record.updated_at = now.clone(); - record.runtime_pid = Some(child.id()); + record.runtime_pid = Some(process.child.id()); record.last_started_at = Some(now); record.last_stopped_at = None; record.last_exit_code = None; record.last_error = None; - runtimes.insert( - record.pubkey.clone(), - ManagedAgentProcess { child, log_path }, - ); + runtimes.insert(record.pubkey.clone(), process); Ok(()) } @@ -1838,11 +1864,20 @@ pub fn stop_managed_agent_process( }; // On Unix, kill the entire process group via terminate_process. - // On non-Unix, fall back to Child::kill() since terminate_process - // is not implemented there. + // On Windows, drop the Job Object handle (KILL_ON_JOB_CLOSE) so the whole + // harness tree dies — Child::kill() would orphan the agent workers + MCP + // servers. If job assignment failed at spawn, fall back to Child::kill(). #[cfg(unix)] terminate_process(runtime.child.id())?; - #[cfg(not(unix))] + #[cfg(windows)] + match runtime.job.take() { + Some(job) => drop(job), + None => runtime + .child + .kill() + .map_err(|error| format!("failed to kill agent process: {error}"))?, + } + #[cfg(not(any(unix, windows)))] runtime .child .kill() diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index e9d733e54..51761a0f9 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -207,6 +207,12 @@ pub struct RelayMeshConfig { pub struct ManagedAgentProcess { pub child: Child, pub log_path: PathBuf, + /// Win32 Job Object owning the harness + its entire process tree. Closing + /// the handle (via `JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE`) kills the whole + /// tree — the Windows mirror of the Unix process-group teardown. `None` + /// if job creation/assignment failed (we fall back to `Child::kill()`). + #[cfg(windows)] + pub job: Option, } #[derive(Debug, Clone, Serialize)] @@ -545,7 +551,6 @@ pub struct MigrationReport { } pub const DEFAULT_ACP_COMMAND: &str = "buzz-acp"; -pub const DEFAULT_AGENT_COMMAND: &str = "goose"; /// ~5 min (320s) — matches the CLI harness default (BUZZ_ACP_IDLE_TIMEOUT). pub const DEFAULT_AGENT_TURN_TIMEOUT_SECONDS: u64 = 320; /// 1 hour — absolute wall-clock safety cap per turn. diff --git a/desktop/src-tauri/src/migration.rs b/desktop/src-tauri/src/migration.rs index 48df3af84..0387b31a4 100644 --- a/desktop/src-tauri/src/migration.rs +++ b/desktop/src-tauri/src/migration.rs @@ -361,7 +361,11 @@ pub fn sync_shared_agent_data(app: &tauri::AppHandle) { } fn reconcile_team_dirs_in_file(path: &Path, target_dir: &Path) { - let target_teams = target_dir.join("agents/teams"); + // Build per-component so the persisted value uses native separators on + // every platform, matching fresh writes (agents.rs builds the same path as + // base.join("teams").join(id)). A single join("agents/teams") would embed a + // literal '/' on Windows, persisting a mixed-separator path into the store. + let target_teams = target_dir.join("agents").join("teams"); patch_json_records(path, |obj| { // Handle both old field name and new field name let field_name = if obj.contains_key("persona_team_dir") { diff --git a/desktop/src-tauri/src/migration_team_dir_tests.rs b/desktop/src-tauri/src/migration_team_dir_tests.rs index afeca0b69..1bad6a9e3 100644 --- a/desktop/src-tauri/src/migration_team_dir_tests.rs +++ b/desktop/src-tauri/src/migration_team_dir_tests.rs @@ -11,17 +11,13 @@ fn team_dir_reconcile_rewrites_worktree_path() { // Team must exist on disk for the reconcile to proceed past the existence gate. std::fs::create_dir_all(canonical.join("agents/teams/com.wpfleger.sietch-tabr")).unwrap(); - let worktree_pack_path = format!( - "{}/agents/packs/com.wpfleger.sietch-tabr", - parent + let worktree_pack_path = pack_dir( + &parent .path() - .join("xyz.block.buzz.app.dev.worktree-my-branch") - .display() - ); - let expected_path = format!( - "{}/agents/teams/com.wpfleger.sietch-tabr", - canonical.display() + .join("xyz.block.buzz.app.dev.worktree-my-branch"), + "com.wpfleger.sietch-tabr", ); + let expected_path = team_dir(&canonical, "com.wpfleger.sietch-tabr"); write_agents_json( &canonical, @@ -47,17 +43,13 @@ fn team_dir_reconcile_rewrites_new_field_name() { // Team must exist on disk for the reconcile to proceed past the existence gate. std::fs::create_dir_all(canonical.join("agents/teams/com.wpfleger.sietch-tabr")).unwrap(); - let worktree_team_path = format!( - "{}/agents/teams/com.wpfleger.sietch-tabr", - parent + let worktree_team_path = team_dir( + &parent .path() - .join("xyz.block.buzz.app.dev.worktree-my-branch") - .display() - ); - let expected_path = format!( - "{}/agents/teams/com.wpfleger.sietch-tabr", - canonical.display() + .join("xyz.block.buzz.app.dev.worktree-my-branch"), + "com.wpfleger.sietch-tabr", ); + let expected_path = team_dir(&canonical, "com.wpfleger.sietch-tabr"); write_agents_json( &canonical, @@ -79,10 +71,7 @@ fn team_dir_reconcile_leaves_canonical_path_unchanged() { let canonical = parent.path().join(CANONICAL_DEV_IDENTIFIER); std::fs::create_dir_all(canonical.join("agents")).unwrap(); - let canonical_path = format!( - "{}/agents/teams/com.wpfleger.sietch-tabr", - canonical.display() - ); + let canonical_path = team_dir(&canonical, "com.wpfleger.sietch-tabr"); write_agents_json( &canonical, @@ -128,12 +117,11 @@ fn team_dir_reconcile_is_idempotent() { // Team must exist on disk for the reconcile to proceed past the existence gate. std::fs::create_dir_all(canonical.join("agents/teams/com.wpfleger.sietch-tabr")).unwrap(); - let worktree_pack_path = format!( - "{}/agents/packs/com.wpfleger.sietch-tabr", - parent + let worktree_pack_path = pack_dir( + &parent .path() - .join("xyz.block.buzz.app.dev.worktree-my-branch") - .display() + .join("xyz.block.buzz.app.dev.worktree-my-branch"), + "com.wpfleger.sietch-tabr", ); write_agents_json( @@ -161,11 +149,11 @@ fn team_dir_reconcile_heals_legacy_release_path() { let release_dir = parent.path().join("xyz.block.buzz.app"); std::fs::create_dir_all(release_dir.join("agents/teams/com.example.team")).unwrap(); - let legacy_path = format!( - "{}/agents/packs/com.example.team", - parent.path().join("xyz.block.sprout.app").display() + let legacy_path = pack_dir( + &parent.path().join("xyz.block.sprout.app"), + "com.example.team", ); - let expected_path = format!("{}/agents/teams/com.example.team", release_dir.display()); + let expected_path = team_dir(&release_dir, "com.example.team"); write_agents_json( &release_dir, @@ -193,12 +181,9 @@ fn team_dir_reconcile_leaves_record_when_team_missing() { std::fs::create_dir_all(canonical.join("agents")).unwrap(); // Intentionally do NOT create agents/teams/com.example.missing. - let stale_path = format!( - "{}/agents/packs/com.example.missing", - parent - .path() - .join("xyz.block.buzz.app.dev.worktree-old") - .display() + let stale_path = pack_dir( + &parent.path().join("xyz.block.buzz.app.dev.worktree-old"), + "com.example.missing", ); write_agents_json( @@ -222,6 +207,7 @@ fn team_dir_reconcile_leaves_record_when_team_missing() { assert!(records[0].get("persona_team_dir").is_none()); } +#[cfg(unix)] #[test] fn team_dir_reconcile_heals_to_symlinked_team_dir() { // When agents/teams/ is a symlink to a real directory elsewhere, @@ -237,14 +223,11 @@ fn team_dir_reconcile_heals_to_symlinked_team_dir() { std::fs::create_dir_all(canonical.join("agents/teams")).unwrap(); std::os::unix::fs::symlink(&real_team, &symlink_target).unwrap(); - let stale_path = format!( - "{}/agents/packs/com.example.team", - parent - .path() - .join("xyz.block.buzz.app.dev.worktree-old") - .display() + let stale_path = pack_dir( + &parent.path().join("xyz.block.buzz.app.dev.worktree-old"), + "com.example.team", ); - let expected_path = format!("{}/agents/teams/com.example.team", canonical.display()); + let expected_path = team_dir(&canonical, "com.example.team"); write_agents_json( &canonical, @@ -260,6 +243,7 @@ fn team_dir_reconcile_heals_to_symlinked_team_dir() { assert_eq!(records[0]["persona_team_dir"], expected_path); } +#[cfg(unix)] #[test] fn team_dir_reconcile_skips_dangling_candidate_symlink() { // When agents/teams/ is a symlink whose target does not exist, @@ -273,12 +257,9 @@ fn team_dir_reconcile_skips_dangling_candidate_symlink() { let symlink_path = canonical.join("agents/teams/com.example.gone"); std::os::unix::fs::symlink(&dangling_target, &symlink_path).unwrap(); - let stale_path = format!( - "{}/agents/packs/com.example.gone", - parent - .path() - .join("xyz.block.buzz.app.dev.worktree-old") - .display() + let stale_path = pack_dir( + &parent.path().join("xyz.block.buzz.app.dev.worktree-old"), + "com.example.gone", ); write_agents_json( @@ -299,6 +280,7 @@ fn team_dir_reconcile_skips_dangling_candidate_symlink() { assert_eq!(records[0]["persona_pack_path"], stale_path); } +#[cfg(unix)] #[test] fn team_dir_reconcile_through_symlink_preserves_symlink() { // Dev worktree instances reach the canonical store through a symlinked @@ -312,8 +294,8 @@ fn team_dir_reconcile_through_symlink_preserves_symlink() { std::fs::create_dir_all(canonical.join("agents/teams/com.example.team")).unwrap(); std::fs::create_dir_all(worktree.join("agents")).unwrap(); - let stale_path = format!("{}/agents/packs/com.example.team", worktree.display()); - let expected_path = format!("{}/agents/teams/com.example.team", canonical.display()); + let stale_path = pack_dir(&worktree, "com.example.team"); + let expected_path = team_dir(&canonical, "com.example.team"); write_agents_json( &canonical, @@ -435,7 +417,7 @@ fn team_dir_reconcile_renames_legacy_field_when_value_already_canonical() { let canonical = parent.path().join(CANONICAL_DEV_IDENTIFIER); std::fs::create_dir_all(canonical.join("agents/teams/com.example.team")).unwrap(); - let correct_path = format!("{}/agents/teams/com.example.team", canonical.display()); + let correct_path = team_dir(&canonical, "com.example.team"); write_agents_json( &canonical, diff --git a/desktop/src-tauri/src/migration_test_support.rs b/desktop/src-tauri/src/migration_test_support.rs index 64a428949..399cb9c8f 100644 --- a/desktop/src-tauri/src/migration_test_support.rs +++ b/desktop/src-tauri/src/migration_test_support.rs @@ -2,6 +2,27 @@ use std::path::Path; +/// Build the native-separator `/agents/teams/` string the way +/// production does (per-component `Path::join`), so test expectations match +/// reconcile output on Windows as well as Unix. +pub(crate) fn team_dir(base: &Path, id: &str) -> String { + base.join("agents") + .join("teams") + .join(id) + .display() + .to_string() +} + +/// Native-separator `/agents/packs/` — the pre-migration layout used +/// as reconcile input. See [`team_dir`] for why per-component join matters. +pub(crate) fn pack_dir(base: &Path, id: &str) -> String { + base.join("agents") + .join("packs") + .join(id) + .display() + .to_string() +} + pub(crate) fn write_agents_json(dir: &Path, records: &serde_json::Value) { std::fs::create_dir_all(dir.join("agents")).unwrap(); std::fs::write( diff --git a/desktop/src-tauri/src/migration_tests.rs b/desktop/src-tauri/src/migration_tests.rs index fefce641f..b414383c1 100644 --- a/desktop/src-tauri/src/migration_tests.rs +++ b/desktop/src-tauri/src/migration_tests.rs @@ -65,6 +65,7 @@ fn copy_dir_all_preserves_nested_files_without_overwriting() { /// Helper: create a temp dir structure mimicking canonical + worktree layout. /// Packs live in a `.main` sibling (not canonical) to match real-world state. /// Returns `(parent_dir_handle, canonical_dir, worktree_dir)`. +#[cfg(unix)] fn setup_sync_layout() -> (tempfile::TempDir, PathBuf, PathBuf) { let parent = tempfile::tempdir().unwrap(); let canonical = parent.path().join(CANONICAL_DEV_IDENTIFIER); @@ -97,6 +98,7 @@ fn setup_sync_layout() -> (tempfile::TempDir, PathBuf, PathBuf) { /// Mirrors the symlink loop of `sync_shared_agent_data` but takes explicit /// paths. `sync_shared_agent_data` requires a live Tauri AppHandle and /// cannot be unit-tested directly. +#[cfg(unix)] fn sync_files(canonical: &Path, worktree: &Path) -> u32 { let mut synced = 0u32; for rel in SHARED_AGENT_FILES { @@ -180,6 +182,7 @@ fn sync_files(canonical: &Path, worktree: &Path) -> u32 { synced } +#[cfg(unix)] #[test] fn sync_creates_symlinks_to_fresh_worktree() { let (_parent, canonical, worktree) = setup_sync_layout(); @@ -201,6 +204,7 @@ fn sync_creates_symlinks_to_fresh_worktree() { ); } +#[cfg(unix)] #[test] fn sync_replaces_existing_files_with_symlinks() { let (_parent, canonical, worktree) = setup_sync_layout(); @@ -226,6 +230,7 @@ fn sync_replaces_existing_files_with_symlinks() { ); } +#[cfg(unix)] #[test] fn sync_preserves_correct_symlinks() { let (_parent, canonical, worktree) = setup_sync_layout(); @@ -238,6 +243,7 @@ fn sync_preserves_correct_symlinks() { } } +#[cfg(unix)] #[test] fn sync_replaces_wrong_symlinks() { let (_parent, canonical, worktree) = setup_sync_layout(); @@ -256,6 +262,7 @@ fn sync_replaces_wrong_symlinks() { } } +#[cfg(unix)] #[test] fn sync_handles_broken_symlinks() { let (_parent, canonical, worktree) = setup_sync_layout(); @@ -275,6 +282,7 @@ fn sync_handles_broken_symlinks() { } } +#[cfg(unix)] #[test] fn writes_through_symlink_reach_canonical() { let (_parent, canonical, worktree) = setup_sync_layout(); @@ -321,6 +329,7 @@ fn canonical_dev_data_dir_returns_self_for_canonical_instance() { assert_eq!(canonical_dev_data_dir(&canonical).unwrap(), canonical); } +#[cfg(unix)] #[test] fn sync_creates_teams_directory_symlink() { let (_parent, canonical, worktree) = setup_sync_layout(); @@ -341,6 +350,7 @@ fn sync_creates_teams_directory_symlink() { ); } +#[cfg(unix)] #[test] fn sync_migrates_teams_from_sibling_to_canonical() { let (_parent, canonical, worktree) = setup_sync_layout(); @@ -368,6 +378,7 @@ fn sync_migrates_teams_from_sibling_to_canonical() { ); } +#[cfg(unix)] #[test] fn sync_replaces_real_teams_dir_with_symlink() { let (_parent, canonical, worktree) = setup_sync_layout(); @@ -386,6 +397,7 @@ fn sync_replaces_real_teams_dir_with_symlink() { // ── Packs → Teams migration tests ─────────────────────────────────── +#[cfg(unix)] #[test] fn migrate_packs_merge_preserves_non_empty_dir() { // When packs/ contains symlinks that weren't moved (e.g., external tools diff --git a/desktop/src/features/settings/UpdateChecker.tsx b/desktop/src/features/settings/UpdateChecker.tsx index 4d524760f..1c43c2097 100644 --- a/desktop/src/features/settings/UpdateChecker.tsx +++ b/desktop/src/features/settings/UpdateChecker.tsx @@ -59,6 +59,21 @@ export function UpdateChecker() { )} + {status.state === "unavailable" && ( + +
+

Update status

+

+ Automatic updates aren't available on this build. Download the + latest release manually. +

+
+ + + )} + {status.state === "available" && (
diff --git a/desktop/src/features/settings/hooks/use-updater.ts b/desktop/src/features/settings/hooks/use-updater.ts index 3b9c5e632..45aff4262 100644 --- a/desktop/src/features/settings/hooks/use-updater.ts +++ b/desktop/src/features/settings/hooks/use-updater.ts @@ -6,6 +6,7 @@ export type UpdateStatus = | { state: "idle" } | { state: "checking" } | { state: "up-to-date" } + | { state: "unavailable" } | { state: "available"; version: string } | { state: "downloading" } | { state: "installing" } @@ -137,8 +138,12 @@ export function useUpdater() { !background || manualResultRequestedRef.current; if (isUpdaterUnavailable(message)) { + // Surface which branch fired on Windows builds where the updater + // plugin is missing — distinguishes plugin-unavailable from a genuine + // up-to-date result in Will's app log. + console.warn(`updater unavailable: ${message}`); if (shouldShowQuietResult) { - setStatus({ state: "idle" }); + setStatus({ state: "unavailable" }); } return; }