diff --git a/docs/next/CHANGELOG.md b/docs/next/CHANGELOG.md index 7acaf007..f17668dd 100644 --- a/docs/next/CHANGELOG.md +++ b/docs/next/CHANGELOG.md @@ -17,6 +17,7 @@ - Known-agent integrations now leave pane ownership to confirmed process exit, so restarting Pi with the same saved session restores lifecycle state even with custom working UI. (#1792) - OMP integration install, status, and uninstall now respect `PI_CONFIG_DIR` when `PI_CODING_AGENT_DIR` is not set, and installation refuses extension-directory collisions with Pi. (#1696) - Physical Escape key records on native Windows now bypass raw VT report framing, so pane applications receive Escape immediately and reliably. (#1736) +- Windows now shows `system` notifications and completes MP3 notification sounds without leaving PowerShell players waiting for a timeout. (#1330) ## [0.7.5] - 2026-07-21 diff --git a/src/platform/windows.rs b/src/platform/windows.rs index 2caf116d..a206b0d1 100644 --- a/src/platform/windows.rs +++ b/src/platform/windows.rs @@ -42,10 +42,13 @@ use windows_sys::{ KEYEVENTF_KEYUP, }, }, - Shell::{CommandLineToArgvW, ShellExecuteW}, + Shell::{ + CommandLineToArgvW, ShellExecuteW, Shell_NotifyIconW, NIF_ICON, NIF_INFO, NIF_TIP, + NIIF_INFO, NIIF_NOSOUND, NIM_ADD, NIM_DELETE, NIM_MODIFY, NOTIFYICONDATAW, + }, WindowsAndMessaging::{ - GetForegroundWindow, GetWindowThreadProcessId, SendMessageTimeoutW, - SMTO_ABORTIFHUNG, WM_IME_CONTROL, + CreateWindowExW, DestroyWindow, GetForegroundWindow, GetWindowThreadProcessId, + LoadIconW, SendMessageTimeoutW, IDI_APPLICATION, SMTO_ABORTIFHUNG, WM_IME_CONTROL, }, }, }, @@ -690,8 +693,110 @@ pub fn read_clipboard_image() -> Option { None } -pub fn show_desktop_notification(_title: &str, _body: Option<&str>) -> std::io::Result { - Ok(false) +pub fn show_desktop_notification(title: &str, body: Option<&str>) -> std::io::Result { + let title = title.to_owned(); + let body = body.unwrap_or(&title).to_owned(); + let (ready_tx, ready_rx) = std::sync::mpsc::sync_channel(1); + std::thread::Builder::new() + .name("herdr-windows-notification".into()) + .spawn(move || show_desktop_notification_on_thread(&title, &body, ready_tx))?; + ready_rx + .recv_timeout(Duration::from_secs(2)) + .map_err(|err| match err { + std::sync::mpsc::RecvTimeoutError::Timeout => std::io::Error::new( + std::io::ErrorKind::TimedOut, + "Windows notification setup timed out", + ), + std::sync::mpsc::RecvTimeoutError::Disconnected => std::io::Error::other( + "Windows notification thread exited before reporting readiness", + ), + })? +} + +fn show_desktop_notification_on_thread( + title: &str, + body: &str, + ready_tx: std::sync::mpsc::SyncSender>, +) { + let class_name = wide_null("STATIC"); + let window_name = wide_null("Herdr notifications"); + let hwnd = unsafe { + CreateWindowExW( + 0, + class_name.as_ptr(), + window_name.as_ptr(), + 0, + 0, + 0, + 0, + 0, + null_mut(), + null_mut(), + null_mut(), + std::ptr::null(), + ) + }; + if hwnd.is_null() { + let _ = ready_tx.send(Err(std::io::Error::last_os_error())); + return; + } + + let mut notification = unsafe { std::mem::zeroed::() }; + notification.cbSize = size_of::() as u32; + notification.hWnd = hwnd; + notification.uID = 1; + notification.hIcon = unsafe { LoadIconW(null_mut(), IDI_APPLICATION) }; + notification.uFlags = NIF_TIP; + if !notification.hIcon.is_null() { + notification.uFlags |= NIF_ICON; + } + copy_wide_truncated(&mut notification.szTip, "Herdr"); + + if unsafe { Shell_NotifyIconW(NIM_ADD, ¬ification) } == 0 { + let _ = ready_tx.send(Err(std::io::Error::other( + "failed to add Herdr notification-area icon", + ))); + unsafe { + DestroyWindow(hwnd); + } + return; + } + + notification.uFlags = NIF_INFO; + notification.dwInfoFlags = NIIF_INFO | NIIF_NOSOUND; + copy_wide_truncated(&mut notification.szInfoTitle, title); + copy_wide_truncated(&mut notification.szInfo, body); + if unsafe { Shell_NotifyIconW(NIM_MODIFY, ¬ification) } == 0 { + unsafe { + Shell_NotifyIconW(NIM_DELETE, ¬ification); + DestroyWindow(hwnd); + } + let _ = ready_tx.send(Err(std::io::Error::other( + "failed to show Herdr desktop notification", + ))); + return; + } + + let _ = ready_tx.send(Ok(true)); + std::thread::sleep(Duration::from_secs(10)); + unsafe { + Shell_NotifyIconW(NIM_DELETE, ¬ification); + DestroyWindow(hwnd); + } +} + +fn copy_wide_truncated(destination: &mut [u16; N], value: &str) { + destination.fill(0); + let mut offset = 0; + for ch in value.chars() { + let mut units = [0; 2]; + let encoded = ch.encode_utf16(&mut units); + if offset + encoded.len() >= N { + break; + } + destination[offset..offset + encoded.len()].copy_from_slice(encoded); + offset += encoded.len(); + } } fn wide_null(value: &str) -> Vec { @@ -1126,6 +1231,15 @@ mod tests { AllocConsole, FreeConsole, GetConsoleProcessList, GetConsoleWindow, }; + #[test] + fn windows_notification_text_is_null_terminated_and_unicode_safe() { + let mut destination = [u16::MAX; 6]; + super::copy_wide_truncated(&mut destination, "abc😀def"); + + assert_eq!(String::from_utf16(&destination[..5]).unwrap(), "abc😀"); + assert_eq!(destination[5], 0); + } + #[test] fn cmd_agent_command_encodes_edge_arguments_without_cmd_expansion() { use base64::Engine as _; diff --git a/src/sound.rs b/src/sound.rs index d55fd084..676dcce1 100644 --- a/src/sound.rs +++ b/src/sound.rs @@ -8,9 +8,7 @@ use std::io::Write; #[cfg(not(any(windows, target_os = "macos")))] use std::io::{Read, Result as IoResult}; use std::path::{Path, PathBuf}; -#[cfg(not(windows))] -use std::process::Command; -use std::process::Output; +use std::process::{Command, Output}; use std::sync::atomic::{AtomicU64, Ordering}; #[cfg(not(any(windows, target_os = "macos")))] use std::time::{Duration, Instant}; @@ -18,6 +16,8 @@ use std::time::{Duration, Instant}; use tracing::warn; const DISABLE_SOUND_ENV: &str = "HERDR_DISABLE_SOUND"; +#[cfg(any(windows, test))] +const WINDOWS_SOUND_PATH_ENV: &str = "HERDR_SOUND_PATH"; #[cfg(not(any(windows, target_os = "macos")))] const AUDIO_PLAYER_TIMEOUT: Duration = Duration::from_secs(15); #[cfg(not(any(windows, target_os = "macos")))] @@ -72,7 +72,7 @@ fn sound_playback_disabled_by_env() -> bool { fn play_file(path: &Path) -> Result<(), String> { match run_player(path) { Ok(output) if output.status.success() => Ok(()), - Ok(output) => Err(format!("player exited with {}", output.status)), + Ok(output) => Err(playback_error(&output)), Err(err) => Err(err), } } @@ -90,11 +90,21 @@ fn play_bytes(data: &[u8]) -> Result<(), String> { match result { Ok(output) if output.status.success() => Ok(()), - Ok(output) => Err(format!("player exited with {}", output.status)), + Ok(output) => Err(playback_error(&output)), Err(e) => Err(e), } } +fn playback_error(output: &Output) -> String { + let stderr = String::from_utf8_lossy(&output.stderr); + let stderr = stderr.trim(); + if stderr.is_empty() { + format!("player exited with {}", output.status) + } else { + format!("player exited with {}: {stderr}", output.status) + } +} + fn temp_sound_path() -> PathBuf { let id = SOUND_TMP_COUNTER.fetch_add(1, Ordering::Relaxed); std::env::temp_dir().join(format!("herdr-sound-{}-{id}.mp3", std::process::id())) @@ -121,38 +131,46 @@ fn run_player(path: &Path) -> Result { #[cfg(any(windows, test))] fn windows_media_player_script() -> &'static str { r#" -param([string]$Path) $ErrorActionPreference = 'Stop' +$Path = [Environment]::GetEnvironmentVariable('HERDR_SOUND_PATH', 'Process') +if ([string]::IsNullOrWhiteSpace($Path)) { throw 'HERDR_SOUND_PATH is not set' } Add-Type -AssemblyName PresentationCore +Add-Type -AssemblyName WindowsBase $resolved = (Resolve-Path -LiteralPath $Path).ProviderPath -$player = [System.Windows.Media.MediaPlayer]::new() -$script:done = $false +$script:player = [System.Windows.Media.MediaPlayer]::new() +$script:frame = [System.Windows.Threading.DispatcherFrame]::new() +$script:timer = [System.Windows.Threading.DispatcherTimer]::new() +$script:timer.Interval = [TimeSpan]::FromSeconds(15) $script:failed = $null -$player.add_MediaEnded({ $script:done = $true }) -$player.add_MediaFailed({ +$script:timedOut = $false +$script:player.add_MediaOpened({ $script:player.Play() }) +$script:player.add_MediaEnded({ $script:frame.Continue = $false }) +$script:player.add_MediaFailed({ param($sender, $eventArgs) $script:failed = $eventArgs.ErrorException - $script:done = $true + $script:frame.Continue = $false }) -$player.Open([Uri]::new($resolved)) -$deadline = [DateTime]::UtcNow.AddSeconds(15) -while (-not $script:done -and -not $player.NaturalDuration.HasTimeSpan -and [DateTime]::UtcNow -lt $deadline) { - Start-Sleep -Milliseconds 25 +$script:timer.add_Tick({ + $script:timedOut = $true + $script:frame.Continue = $false +}) +try { + $script:player.Open([Uri]::new($resolved)) + $script:timer.Start() + [System.Windows.Threading.Dispatcher]::PushFrame($script:frame) +} finally { + $script:timer.Stop() + $script:player.Close() } -if ($script:failed) { throw $script:failed } -$player.Play() -while (-not $script:done -and [DateTime]::UtcNow -lt $deadline) { - Start-Sleep -Milliseconds 50 -} -$player.Close() -if ($script:failed) { throw $script:failed } -if (-not $script:done) { throw 'sound playback timed out' } +if ($script:failed) { throw "sound media failed: $($script:failed.Message)" } +if ($script:timedOut) { throw 'sound playback timed out' } "# } -#[cfg(windows)] -fn run_windows_player(path: &Path) -> Result { - crate::noninteractive_process::command("powershell.exe") +#[cfg(any(windows, test))] +fn windows_player_command(path: &Path) -> Command { + let mut command = crate::noninteractive_process::command("powershell.exe"); + command .args([ "-NoLogo", "-NoProfile", @@ -162,7 +180,13 @@ fn run_windows_player(path: &Path) -> Result { "-Command", windows_media_player_script(), ]) - .arg(path) + .env(WINDOWS_SOUND_PATH_ENV, path); + command +} + +#[cfg(windows)] +fn run_windows_player(path: &Path) -> Result { + windows_player_command(path) .output() .map_err(|e| format!("Windows MediaPlayer playback failed: {e}")) } @@ -403,11 +427,43 @@ mod tests { } #[test] - fn windows_media_player_script_accepts_literal_path_argument() { + fn windows_media_player_uses_process_environment_and_dispatcher() { let script = windows_media_player_script(); + let path = Path::new(r"C:\sound dir\döne.mp3"); + let command = windows_player_command(path); + let env_path = command.get_envs().find_map(|(key, value)| { + (key == std::ffi::OsStr::new(WINDOWS_SOUND_PATH_ENV)) + .then_some(value) + .flatten() + }); - assert!(script.contains("param([string]$Path)")); + assert!(script.contains("GetEnvironmentVariable('HERDR_SOUND_PATH', 'Process')")); + assert!(!script.contains("param([string]$Path)")); assert!(script.contains("Resolve-Path -LiteralPath $Path")); - assert!(script.contains("System.Windows.Media.MediaPlayer")); + assert!(script.contains("Dispatcher]::PushFrame")); + assert!(script.contains("add_MediaEnded")); + assert!(script.contains("add_MediaFailed")); + assert_eq!(env_path, Some(path.as_os_str())); + assert!(!command.get_args().any(|arg| arg == path.as_os_str())); + } + + #[cfg(windows)] + #[test] + fn windows_media_player_reports_invalid_media_without_waiting_for_timeout() { + let path = temp_sound_path(); + std::fs::write(&path, b"not an mp3").unwrap(); + let started = std::time::Instant::now(); + let output = run_windows_player(&path).unwrap(); + let _ = std::fs::remove_file(path); + + assert!(!output.status.success()); + assert!( + started.elapsed() < std::time::Duration::from_secs(5), + "MediaFailed should stop playback promptly" + ); + assert!( + String::from_utf8_lossy(&output.stderr).contains("sound media failed"), + "stderr should identify a MediaFailed error" + ); } }