fix(windows): restore system notifications and sound playback (#2019)
This commit is contained in:
Can Celik
GitHub
parent
73d92004f5
commit
5b0be42e04
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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<ClipboardImage> {
|
|||
None
|
||||
}
|
||||
|
||||
pub fn show_desktop_notification(_title: &str, _body: Option<&str>) -> std::io::Result<bool> {
|
||||
Ok(false)
|
||||
pub fn show_desktop_notification(title: &str, body: Option<&str>) -> std::io::Result<bool> {
|
||||
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<std::io::Result<bool>>,
|
||||
) {
|
||||
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::<NOTIFYICONDATAW>() };
|
||||
notification.cbSize = size_of::<NOTIFYICONDATAW>() 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<const N: usize>(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<u16> {
|
||||
|
|
@ -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 _;
|
||||
|
|
|
|||
116
src/sound.rs
116
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<Output, String> {
|
|||
#[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<Output, String> {
|
||||
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<Output, String> {
|
|||
"-Command",
|
||||
windows_media_player_script(),
|
||||
])
|
||||
.arg(path)
|
||||
.env(WINDOWS_SOUND_PATH_ENV, path);
|
||||
command
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
fn run_windows_player(path: &Path) -> Result<Output, String> {
|
||||
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"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue