From f7f49151d53e4ada27f55ae05a483824ee36b4b1 Mon Sep 17 00:00:00 2001 From: Anbazhagan T Date: Mon, 18 May 2026 17:36:55 -0700 Subject: [PATCH 1/2] fix: improve Windows compatibility for daemon telemetry and hook commands --- src/daemon/telemetry_handle.rs | 12 ++++++++++- src/mdm/agents/codex.rs | 36 ++++++++++++++++++++++++++++---- src/mdm/agents/cursor.rs | 29 ++++++++++++------------- src/mdm/agents/droid.rs | 11 ++++++---- src/mdm/agents/firebender.rs | 15 ++++--------- src/mdm/agents/gemini.rs | 12 +++++------ src/mdm/agents/github_copilot.rs | 33 +++++++++++++---------------- src/mdm/agents/windsurf.rs | 5 +++-- src/mdm/ensure_git_symlinks.rs | 16 ++++++++++---- src/mdm/utils.rs | 12 +++++++++++ 10 files changed, 113 insertions(+), 68 deletions(-) diff --git a/src/daemon/telemetry_handle.rs b/src/daemon/telemetry_handle.rs index 6d54da60a1..96a7b1237b 100644 --- a/src/daemon/telemetry_handle.rs +++ b/src/daemon/telemetry_handle.rs @@ -23,7 +23,17 @@ use std::time::Duration; const DAEMON_SOCKET_IO_TIMEOUT: Duration = Duration::from_secs(2); /// Maximum time to wait for the daemon socket on process start. -#[cfg(not(any(test, feature = "test-support")))] +/// +/// Windows Named Pipes require a listening server instance before `connect_ms` +/// returns, and process startup on Windows is meaningfully slower than on Unix +/// (AV scanning, loader lock, etc.). The daemon startup timeout in +/// `daemon_startup_timeout()` already grants 5 s on Windows for this reason; +/// we mirror that budget here so the git wrapper does not time-out connecting +/// to the control socket before the daemon is ready. +#[cfg(all(not(any(test, feature = "test-support")), windows))] +const DAEMON_TELEMETRY_CONNECT_TIMEOUT: Duration = Duration::from_secs(5); + +#[cfg(all(not(any(test, feature = "test-support")), not(windows)))] const DAEMON_TELEMETRY_CONNECT_TIMEOUT: Duration = Duration::from_secs(2); /// Global handle to the daemon control socket for telemetry submission. diff --git a/src/mdm/agents/codex.rs b/src/mdm/agents/codex.rs index 8bce94eee0..655581eb2a 100644 --- a/src/mdm/agents/codex.rs +++ b/src/mdm/agents/codex.rs @@ -1,7 +1,8 @@ use crate::error::GitAiError; use crate::mdm::hook_installer::{HookCheckResult, HookInstaller, HookInstallerParams}; use crate::mdm::utils::{ - binary_exists, generate_diff, home_dir, is_git_ai_checkpoint_command, write_atomic, + binary_exists, generate_diff, home_dir, is_git_ai_checkpoint_command, + to_agent_hook_command_path, to_git_bash_path, write_atomic, }; use serde_json::{Value as JsonValue, json}; use sha2::{Digest, Sha256}; @@ -25,7 +26,15 @@ impl CodexInstaller { } fn desired_command(binary_path: &Path) -> String { - format!("{} {}", binary_path.display(), CODEX_CHECKPOINT_CMD) + format!( + "{} {}", + to_agent_hook_command_path(binary_path), + CODEX_CHECKPOINT_CMD + ) + } + + fn legacy_msys_desired_command(binary_path: &Path) -> String { + format!("{} {}", to_git_bash_path(binary_path), CODEX_CHECKPOINT_CMD) } fn parse_config_toml(content: &str) -> Result { @@ -210,6 +219,13 @@ impl CodexInstaller { fn config_with_installed_hooks( config: &TomlValue, binary_path: &Path, + ) -> Result { + Self::config_with_installed_hooks_for_command(config, Self::desired_command(binary_path)) + } + + fn config_with_installed_hooks_for_command( + config: &TomlValue, + desired_command: String, ) -> Result { let mut merged = Self::remove_notify_if_git_ai(config)?.unwrap_or(config.clone()); let root = merged @@ -231,7 +247,6 @@ impl CodexInstaller { } // Add inline hooks to config.toml under [hooks] table - let desired_command = Self::desired_command(binary_path); let hooks_table = root .entry("hooks") .or_insert_with(|| TomlValue::Table(Map::new())); @@ -631,6 +646,14 @@ impl HookInstaller for CodexInstaller { }; let desired_config = Self::config_with_installed_hooks(&config, ¶ms.binary_path)?; + let legacy_msys_desired_config = if cfg!(windows) { + Some(Self::config_with_installed_hooks_for_command( + &config, + Self::legacy_msys_desired_command(¶ms.binary_path), + )?) + } else { + None + }; let has_inline_hooks = Self::config_has_inline_hooks(&config); let has_legacy_hooks_json = Self::hooks_json_path().exists() && Self::parse_hooks_json(&fs::read_to_string(Self::hooks_json_path())?) @@ -638,7 +661,12 @@ impl HookInstaller for CodexInstaller { .unwrap_or(false); let hooks_installed = Self::config_hooks_feature_enabled(&config) && (has_inline_hooks || has_legacy_hooks_json); - let hooks_up_to_date = config == desired_config && !has_legacy_hooks_json; + let hooks_up_to_date = (config == desired_config + || legacy_msys_desired_config + .as_ref() + .map(|legacy| config == *legacy) + .unwrap_or(false)) + && !has_legacy_hooks_json; Ok(HookCheckResult { tool_installed: true, diff --git a/src/mdm/agents/cursor.rs b/src/mdm/agents/cursor.rs index 78664c6e96..7658866fbc 100644 --- a/src/mdm/agents/cursor.rs +++ b/src/mdm/agents/cursor.rs @@ -4,9 +4,9 @@ use crate::mdm::hook_installer::{ }; use crate::mdm::utils::{ MIN_CURSOR_VERSION, generate_diff, get_editor_version, home_dir, install_vsc_editor_extension, - is_vsc_editor_extension_installed, parse_version, resolve_editor_cli, - settings_paths_for_products, should_process_settings_target, version_meets_requirement, - write_atomic, + is_vsc_editor_extension_installed, parse_version, resolve_editor_cli, settings_paths_for_products, + should_process_settings_target, to_agent_hook_command_path, + version_meets_requirement, write_atomic, }; use serde_json::{Value, json}; use std::fs; @@ -135,16 +135,9 @@ impl HookInstaller for CursorInstaller { }; // Build commands with absolute path - let pre_tool_use_cmd = format!( - "{} {}", - params.binary_path.display(), - CURSOR_PRE_TOOL_USE_CMD - ); - let post_tool_use_cmd = format!( - "{} {}", - params.binary_path.display(), - CURSOR_POST_TOOL_USE_CMD - ); + let binary_path_str = to_agent_hook_command_path(¶ms.binary_path); + let pre_tool_use_cmd = format!("{} {}", binary_path_str, CURSOR_PRE_TOOL_USE_CMD); + let post_tool_use_cmd = format!("{} {}", binary_path_str, CURSOR_POST_TOOL_USE_CMD); // Desired hooks payload for Cursor let desired: Value = json!({ @@ -433,7 +426,7 @@ impl HookInstaller for CursorInstaller { #[cfg(test)] mod tests { use super::*; - use crate::mdm::utils::clean_path; + use crate::mdm::utils::{clean_path, to_agent_hook_command_path}; use std::fs; use tempfile::TempDir; @@ -653,11 +646,15 @@ mod tests { #[test] fn test_cursor_hook_commands_no_windows_extended_path_prefix() { + // Simulate the production path: get_current_binary_path() calls clean_path to strip + // the \\?\ extended-length prefix, then we call to_agent_hook_command_path to + // normalize the command path for hook execution. let raw_path = PathBuf::from(r"\\?\C:\Users\USERNAME\.git-ai\bin\git-ai.exe"); let binary_path = clean_path(raw_path); + let binary_path_str = to_agent_hook_command_path(&binary_path); - let pre_tool_use_cmd = format!("{} {}", binary_path.display(), CURSOR_PRE_TOOL_USE_CMD); - let post_tool_use_cmd = format!("{} {}", binary_path.display(), CURSOR_POST_TOOL_USE_CMD); + let pre_tool_use_cmd = format!("{} {}", binary_path_str, CURSOR_PRE_TOOL_USE_CMD); + let post_tool_use_cmd = format!("{} {}", binary_path_str, CURSOR_POST_TOOL_USE_CMD); assert!( !pre_tool_use_cmd.contains(r"\\?\"), diff --git a/src/mdm/agents/droid.rs b/src/mdm/agents/droid.rs index 42f05d409c..819725fbd2 100644 --- a/src/mdm/agents/droid.rs +++ b/src/mdm/agents/droid.rs @@ -1,6 +1,9 @@ use crate::error::GitAiError; use crate::mdm::hook_installer::{HookCheckResult, HookInstaller, HookInstallerParams}; -use crate::mdm::utils::{generate_diff, home_dir, is_git_ai_checkpoint_command, write_atomic}; +use crate::mdm::utils::{ + generate_diff, home_dir, is_git_ai_checkpoint_command, to_agent_hook_command_path, + write_atomic, +}; use serde_json::{Value, json}; use std::fs; use std::path::{Path, PathBuf}; @@ -84,9 +87,9 @@ impl DroidInstaller { serde_json::from_str(&existing_content)? }; - let binary_path = params.binary_path.to_string_lossy().to_string(); - let pre_tool_cmd = format!("{} {}", binary_path, DROID_PRE_TOOL_CMD); - let post_tool_cmd = format!("{} {}", binary_path, DROID_POST_TOOL_CMD); + let binary_path_str = to_agent_hook_command_path(¶ms.binary_path); + let pre_tool_cmd = format!("{} {}", binary_path_str, DROID_PRE_TOOL_CMD); + let post_tool_cmd = format!("{} {}", binary_path_str, DROID_POST_TOOL_CMD); let mut merged = existing.clone(); let mut hooks_obj = merged.get("hooks").cloned().unwrap_or_else(|| json!({})); diff --git a/src/mdm/agents/firebender.rs b/src/mdm/agents/firebender.rs index 3016d328c0..7df09f3eb9 100644 --- a/src/mdm/agents/firebender.rs +++ b/src/mdm/agents/firebender.rs @@ -1,6 +1,6 @@ use crate::error::GitAiError; use crate::mdm::hook_installer::{HookCheckResult, HookInstaller, HookInstallerParams}; -use crate::mdm::utils::{generate_diff, home_dir, write_atomic}; +use crate::mdm::utils::{generate_diff, home_dir, to_agent_hook_command_path, write_atomic}; use serde_json::{Value, json}; use std::fs; use std::path::PathBuf; @@ -112,16 +112,9 @@ impl HookInstaller for FirebenderInstaller { serde_json::from_str(&existing_content)? }; - let pre_tool_use_cmd = format!( - "{} {}", - params.binary_path.display(), - FIREBENDER_PRE_TOOL_USE_CMD - ); - let post_tool_use_cmd = format!( - "{} {}", - params.binary_path.display(), - FIREBENDER_POST_TOOL_USE_CMD - ); + let binary_path_str = to_agent_hook_command_path(¶ms.binary_path); + let pre_tool_use_cmd = format!("{} {}", binary_path_str, FIREBENDER_PRE_TOOL_USE_CMD); + let post_tool_use_cmd = format!("{} {}", binary_path_str, FIREBENDER_POST_TOOL_USE_CMD); let desired: Value = json!({ "version": 1, diff --git a/src/mdm/agents/gemini.rs b/src/mdm/agents/gemini.rs index a85d6f87e9..8cb41c7d79 100644 --- a/src/mdm/agents/gemini.rs +++ b/src/mdm/agents/gemini.rs @@ -1,7 +1,8 @@ use crate::error::GitAiError; use crate::mdm::hook_installer::{HookCheckResult, HookInstaller, HookInstallerParams}; use crate::mdm::utils::{ - binary_exists, generate_diff, home_dir, is_git_ai_checkpoint_command, write_atomic, + binary_exists, generate_diff, home_dir, is_git_ai_checkpoint_command, + to_agent_hook_command_path, write_atomic, }; use serde_json::{Value, json}; use std::fs; @@ -87,12 +88,9 @@ impl GeminiInstaller { serde_json::from_str(&existing_content)? }; - let before_tool_cmd = format!( - "{} {}", - params.binary_path.display(), - GEMINI_BEFORE_TOOL_CMD - ); - let after_tool_cmd = format!("{} {}", params.binary_path.display(), GEMINI_AFTER_TOOL_CMD); + let binary_path_str = to_agent_hook_command_path(¶ms.binary_path); + let before_tool_cmd = format!("{} {}", binary_path_str, GEMINI_BEFORE_TOOL_CMD); + let after_tool_cmd = format!("{} {}", binary_path_str, GEMINI_AFTER_TOOL_CMD); let mut merged = existing.clone(); diff --git a/src/mdm/agents/github_copilot.rs b/src/mdm/agents/github_copilot.rs index 06b4d97308..43b7e67bc6 100644 --- a/src/mdm/agents/github_copilot.rs +++ b/src/mdm/agents/github_copilot.rs @@ -3,7 +3,7 @@ use crate::mdm::hook_installer::{HookCheckResult, HookInstaller, HookInstallerPa use crate::mdm::utils::{ MIN_CODE_VERSION, generate_diff, get_editor_version, home_dir, parse_version, resolve_editor_cli, settings_paths_for_products, should_process_settings_target, - version_meets_requirement, write_atomic, + to_agent_hook_command_path, to_git_bash_path, version_meets_requirement, write_atomic, }; use serde_json::{Value, json}; use std::fs; @@ -107,15 +107,17 @@ impl HookInstaller for GitHubCopilotInstaller { let content = fs::read_to_string(&hooks_path)?; let existing: Value = serde_json::from_str(&content).unwrap_or_else(|_| json!({})); - let pre_desired = format!( + let binary_path_str = to_agent_hook_command_path(¶ms.binary_path); + let pre_desired = format!("{} {}", binary_path_str, GITHUB_COPILOT_PRE_TOOL_CMD); + let post_desired = format!("{} {}", binary_path_str, GITHUB_COPILOT_POST_TOOL_CMD); + let legacy_msys_binary_path_str = to_git_bash_path(¶ms.binary_path); + let pre_legacy = format!( "{} {}", - params.binary_path.display(), - GITHUB_COPILOT_PRE_TOOL_CMD + legacy_msys_binary_path_str, GITHUB_COPILOT_PRE_TOOL_CMD ); - let post_desired = format!( + let post_legacy = format!( "{} {}", - params.binary_path.display(), - GITHUB_COPILOT_POST_TOOL_CMD + legacy_msys_binary_path_str, GITHUB_COPILOT_POST_TOOL_CMD ); let has_pre_installed = existing @@ -154,7 +156,7 @@ impl HookInstaller for GitHubCopilotInstaller { arr.iter().any(|hook| { hook.get("command") .and_then(|c| c.as_str()) - .map(|cmd| cmd == pre_desired) + .map(|cmd| cmd == pre_desired || (cfg!(windows) && cmd == pre_legacy)) .unwrap_or(false) }) }) @@ -168,7 +170,7 @@ impl HookInstaller for GitHubCopilotInstaller { arr.iter().any(|hook| { hook.get("command") .and_then(|c| c.as_str()) - .map(|cmd| cmd == post_desired) + .map(|cmd| cmd == post_desired || (cfg!(windows) && cmd == post_legacy)) .unwrap_or(false) }) }) @@ -204,16 +206,9 @@ impl HookInstaller for GitHubCopilotInstaller { serde_json::from_str(&existing_content)? }; - let pre_tool_cmd = format!( - "{} {}", - params.binary_path.display(), - GITHUB_COPILOT_PRE_TOOL_CMD - ); - let post_tool_cmd = format!( - "{} {}", - params.binary_path.display(), - GITHUB_COPILOT_POST_TOOL_CMD - ); + let binary_path_str = to_agent_hook_command_path(¶ms.binary_path); + let pre_tool_cmd = format!("{} {}", binary_path_str, GITHUB_COPILOT_PRE_TOOL_CMD); + let post_tool_cmd = format!("{} {}", binary_path_str, GITHUB_COPILOT_POST_TOOL_CMD); let desired: Value = json!({ "hooks": { diff --git a/src/mdm/agents/windsurf.rs b/src/mdm/agents/windsurf.rs index 910fdee6cc..f7379d483e 100644 --- a/src/mdm/agents/windsurf.rs +++ b/src/mdm/agents/windsurf.rs @@ -4,7 +4,8 @@ use crate::mdm::hook_installer::{ }; use crate::mdm::utils::{ generate_diff, home_dir, install_vsc_editor_extension, is_git_ai_checkpoint_command, - is_github_codespaces, is_vsc_editor_extension_installed, resolve_editor_cli, write_atomic, + is_github_codespaces, is_vsc_editor_extension_installed, resolve_editor_cli, + to_agent_hook_command_path, write_atomic, }; use serde_json::{Value, json}; @@ -271,7 +272,7 @@ impl HookInstaller for WindsurfInstaller { ) -> Result, GitAiError> { let desired_cmd = format!( "{} {}", - params.binary_path.display(), + to_agent_hook_command_path(¶ms.binary_path), WINDSURF_CHECKPOINT_CMD ); diff --git a/src/mdm/ensure_git_symlinks.rs b/src/mdm/ensure_git_symlinks.rs index d492949810..bfe7e1b58b 100644 --- a/src/mdm/ensure_git_symlinks.rs +++ b/src/mdm/ensure_git_symlinks.rs @@ -38,12 +38,20 @@ pub fn ensure_git_symlinks() -> Result<(), GitAiError> { // Remove existing symlink/junction if present if symlink_path.exists() || symlink_path.symlink_metadata().is_ok() { - // On Windows, junctions are directories, so use remove_dir + // On Windows, junctions are directories, so try remove_dir first, + // then fall back to remove_file (for regular symlinks). Both errors + // are surfaced rather than swallowed so callers get a clear diagnosis. #[cfg(windows)] { - // Try remove_dir first (for junctions), then remove_file (for symlinks) - if std::fs::remove_dir(&symlink_path).is_err() { - let _ = std::fs::remove_file(&symlink_path); + let dir_err = std::fs::remove_dir(&symlink_path); + if dir_err.is_err() { + std::fs::remove_file(&symlink_path).map_err(|e| { + GitAiError::Generic(format!( + "Failed to remove existing junction/symlink at {}: {}", + symlink_path.display(), + e + )) + })?; } } #[cfg(unix)] diff --git a/src/mdm/utils.rs b/src/mdm/utils.rs index c3cd5bf570..0078ddbbf5 100644 --- a/src/mdm/utils.rs +++ b/src/mdm/utils.rs @@ -704,6 +704,18 @@ pub fn to_git_bash_path(path: &Path) -> String { s.into_owned() } +/// Convert a path for agent hook command execution. +/// On Windows, use `C:/...` style paths so commands can run in cmd.exe, +/// PowerShell, and shells that also accept forward slashes. +/// On non-Windows platforms, return the existing hook command path style. +pub fn to_agent_hook_command_path(path: &Path) -> String { + if cfg!(windows) { + to_windows_git_bash_style_path(path) + } else { + to_git_bash_path(path) + } +} + /// Get the absolute path to the currently running binary pub fn get_current_binary_path() -> Result { let path = std::env::current_exe()?; From ab5f84f1863516a7767f8b37d5bfdd98d205ff60 Mon Sep 17 00:00:00 2001 From: Anbu Date: Mon, 18 May 2026 17:43:14 -0700 Subject: [PATCH 2/2] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/mdm/agents/codex.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mdm/agents/codex.rs b/src/mdm/agents/codex.rs index 655581eb2a..11049da2fa 100644 --- a/src/mdm/agents/codex.rs +++ b/src/mdm/agents/codex.rs @@ -664,7 +664,7 @@ impl HookInstaller for CodexInstaller { let hooks_up_to_date = (config == desired_config || legacy_msys_desired_config .as_ref() - .map(|legacy| config == *legacy) + .map(|legacy| &config == legacy) .unwrap_or(false)) && !has_legacy_hooks_json;