diff --git a/.github/scripts/windows-installer.iss b/.github/scripts/windows-installer.iss index 70afae0b..0e7cf0d2 100644 --- a/.github/scripts/windows-installer.iss +++ b/.github/scripts/windows-installer.iss @@ -120,9 +120,15 @@ Source: "{#StageDir}\server\*"; DestDir: "{app}\server"; Flags: ignoreversion re ; all-users install alone — it cannot write {commonprograms} unelevated, and a ; per-user twin would both duplicate the Start Menu entry and outlive this ; uninstaller — so an elevated install depends on the stamp right here. +; +; AppUserModelToastActivatorCLSID names the COM class a click on a toast in +; notification history activates; without it that click goes nowhere (#1179). +; Must match `core::aumid::TOAST_ACTIVATOR_CLSID`. The app registers the class +; itself (HKCU, at every launch, so it always names the running exe); the +; uninstaller below removes that registration. [Icons] -Name: "{autoprograms}\tty7"; Filename: "{app}\tty7-app.exe"; AppUserModelID: "com.github.tty7" -Name: "{autodesktop}\tty7"; Filename: "{app}\tty7-app.exe"; Tasks: desktopicon; AppUserModelID: "com.github.tty7" +Name: "{autoprograms}\tty7"; Filename: "{app}\tty7-app.exe"; AppUserModelID: "com.github.tty7"; AppUserModelToastActivatorCLSID: "4E1F6381-E90B-4BED-9156-A9013F718FE6" +Name: "{autodesktop}\tty7"; Filename: "{app}\tty7-app.exe"; Tasks: desktopicon; AppUserModelID: "com.github.tty7"; AppUserModelToastActivatorCLSID: "4E1F6381-E90B-4BED-9156-A9013F718FE6" [Run] ; The registry shape lives in core::explorer_context_menu, not here: the app @@ -253,11 +259,25 @@ begin HKEY_CURRENT_USER, 'Software\Classes\Directory\Background\shell\tty7'); end; +(* Remove the toast activator's COM registration. + + `core::aumid` writes HKCU\Software\Classes\CLSID\{…}\LocalServer32 at launch + so that a click on a toast in notification history can start tty7. Left + behind, it would point COM at a deleted exe. Only the class's own key is + deleted; a missing key is a no-op. HKCU for the same reason as the PATH + entry above: that is the only hive the app writes. *) +procedure RemoveToastActivator(); +begin + RegDeleteKeyIncludingSubkeys( + HKEY_CURRENT_USER, 'Software\Classes\CLSID\{4E1F6381-E90B-4BED-9156-A9013F718FE6}'); +end; + procedure CurUninstallStepChanged(CurUninstallStep: TUninstallStep); begin if CurUninstallStep = usUninstall then begin RemoveAppDirFromUserPath(); RemoveExplorerContextMenu(); + RemoveToastActivator(); end; end; diff --git a/Cargo.lock b/Cargo.lock index 6549e03e..12c21fdd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11575,6 +11575,7 @@ dependencies = [ "url", "uuid", "windows 0.58.0", + "windows-core 0.58.0", "windows-sys 0.61.2", "winresource", ] diff --git a/Cargo.toml b/Cargo.toml index 2405f9d6..ca221a20 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -220,7 +220,15 @@ windows = { version = "0.58", features = [ "Win32_System_Variant", "Win32_UI_Shell", "Win32_UI_Shell_PropertiesSystem", + # INotificationActivationCallback, the COM class a click on a toast in + # notification history activates (`core::toast_activator`); `implement` + # is what lets us provide it and its class factory. + "Win32_UI_Notifications", + "implement", ] } +# `#[implement]` expands to paths under `::windows_core`, so the crate has to +# be a direct dependency. Same version `windows` 0.58 already pulls in. +windows-core = "0.58" # Embeds `assets/favicon.ico` into the `.exe` so Windows shows the tty7 logo in # the taskbar / window / Explorer (macOS gets its icon from the `.app` bundle via diff --git a/src/core/aumid.rs b/src/core/aumid.rs index 746a8668..6258d65d 100644 --- a/src/core/aumid.rs +++ b/src/core/aumid.rs @@ -16,6 +16,14 @@ //! anything at all from a `cargo` build directory (which would repoint the //! user's installed shortcut at `target\debug`). //! +//! The same shortcut also names tty7's toast activator +//! (`System.AppUserModel.ToastActivatorCLSID`), and `init()` registers that +//! class as a per-user COM local server. That pair is what makes a click on a +//! toast sitting in notification history reach us: the shell activates the +//! class over COM — launching tty7 when it is not running — instead of firing +//! the `Activated` event of a `ToastNotification` object that is long gone. +//! See `core::toast_activator`. +//! //! Everything is best-effort. `toast_app_id()` yields the AUMID only once a //! shortcut carrying it is in place *and* the shell has had time to index it; //! otherwise callers keep the PowerShell identity, which looks wrong but still @@ -33,6 +41,33 @@ use std::time::{Duration, Instant}; /// mismatch silently splits the identity in two (a unit test checks this). pub(crate) const AUMID: &str = "com.github.tty7"; +/// The toast activator class (`core::toast_activator`). Keep in sync with the +/// `AppUserModelToastActivatorCLSID` on the installer shortcuts and the key +/// its uninstaller removes — a unit test checks both. Never change it: older +/// shortcuts and registrations in the field name this exact class. +pub(crate) const TOAST_ACTIVATOR_CLSID: windows::core::GUID = + windows::core::GUID::from_u128(0x4E1F6381_E90B_4BED_9156_A9013F718FE6); + +/// The argument the COM registration launches us with. COM appends +/// `-Embedding` after it; neither changes how the launch is handled — it is a +/// pathless launch whose window the activation then brings forward — but they +/// make the process's origin legible in a process list or a crash report. +const TOAST_ACTIVATED_ARG: &str = "--toast-activated"; + +/// The CLSID as the registry and the installer spell it, without braces: +/// `4E1F6381-E90B-4BED-9156-A9013F718FE6`. +fn clsid_text(clsid: &windows::core::GUID) -> String { + let hex = format!("{:032X}", clsid.to_u128()); + format!( + "{}-{}-{}-{}-{}", + &hex[..8], + &hex[8..12], + &hex[12..16], + &hex[16..20], + &hex[20..] + ) +} + /// How long we keep using the PowerShell identity after writing the shortcut /// ourselves. The shell picks a new `.lnk` up asynchronously and silently /// discards toasts for an AUMID it has not indexed yet; the measured lag on @@ -42,6 +77,9 @@ const INDEX_GRACE: Duration = Duration::from_secs(30); /// One-time GUI-startup hook; see the module docs. Cheap after the first call. pub(crate) fn init() { + // First, so a launch that COM made to deliver a toast click has its class + // object up as early as possible; COM is waiting on it. + crate::core::toast_activator::start(); let _ = toast_app_id(); } @@ -74,13 +112,17 @@ fn setup() -> Branding { // directories the shortcut half below refuses to touch. let _ = SetCurrentProcessExplicitAppUserModelID(&windows::core::HSTRING::from(AUMID)); } + register_local_server(); match decide() { Ok(Decision::Branded) => Branding::Ready, Ok(Decision::Skip(why)) => { log::debug!("keeping the PowerShell toast identity: {why}"); Branding::Unavailable } - Ok(Decision::Write(lnk)) => match write_shortcut(&lnk) { + Ok(Decision::Write { lnk, indexed }) => match write_shortcut(&lnk) { + // Refreshing a shortcut that already carried the AUMID (to add the + // toast activator) leaves the identity the shell indexed intact. + Ok(()) if indexed => Branding::Ready, Ok(()) => Branding::Pending(Instant::now()), Err(e) => { log::warn!("toast branding disabled, keeping the PowerShell identity: {e}"); @@ -97,8 +139,9 @@ fn setup() -> Branding { enum Decision { /// A shortcut already carries our AUMID; write nothing. Branded, - /// Create or refresh this per-user shortcut, in place. - Write(PathBuf), + /// Create or refresh this per-user shortcut, in place. `indexed` when it + /// already carries our AUMID, so the shell knows the identity already. + Write { lnk: PathBuf, indexed: bool }, /// Nothing we may safely write. The string is for the log. Skip(&'static str), } @@ -135,11 +178,17 @@ fn decide() -> Result { let stamped = existing .as_ref() .is_some_and(|s| s.aumid.as_deref() == Some(AUMID)); + // A shortcut from before the toast activator existed brands toasts fine, + // but a click on one in notification history goes nowhere until the + // shortcut also names the class — so it is rewritten like an off-target one. + let activates = existing + .as_ref() + .is_some_and(|s| s.toast_activator == Some(TOAST_ACTIVATOR_CLSID)); let on_target = existing .as_ref() .and_then(|s| s.target.as_deref()) .is_some_and(|t| same_path(t, &exe)); - if stamped && on_target { + if stamped && activates && on_target { return Ok(Decision::Branded); } @@ -155,7 +204,115 @@ fn decide() -> Result { }); } - Ok(Decision::Write(lnk)) + Ok(Decision::Write { + lnk, + indexed: stamped, + }) +} + +/// `HKCU\Software\Classes\CLSID\{…}\LocalServer32` — what COM runs to +/// deliver a toast click while tty7 is not running. Per-user, so it needs no +/// elevation whatever the install shape; the uninstaller removes the CLSID key. +fn local_server_key() -> String { + format!( + r"Software\Classes\CLSID\{{{}}}\LocalServer32", + clsid_text(&TOAST_ACTIVATOR_CLSID) + ) +} + +fn local_server_command(exe: &Path) -> std::ffi::OsString { + let mut command = std::ffi::OsString::from("\""); + command.push(exe.as_os_str()); + command.push("\" "); + command.push(TOAST_ACTIVATED_ARG); + command +} + +/// Point the toast activator's COM registration at the running exe, unless +/// it already does. Never from a cargo build directory, for the same reason +/// `decide()` never writes the shortcut from one: a `cargo run` must not leave +/// the user's toast clicks launching `target\debug`. A dev build still +/// receives clicks while it is running, through its class object. +fn register_local_server() { + let exe = match std::env::current_exe() { + Ok(exe) => exe, + Err(e) => { + log::warn!("toast activator not registered: current exe: {e}"); + return; + } + }; + if is_build_output(&exe) { + return; + } + let key = local_server_key(); + let command = local_server_command(&exe); + if read_default_string(&key).as_deref() == Some(command.as_os_str()) { + return; + } + if let Err(e) = write_default_string(&key, &command) { + log::warn!("toast activator not registered: {e}"); + } +} + +fn read_default_string(subkey: &str) -> Option { + use std::os::windows::ffi::OsStringExt; + use windows_sys::Win32::Foundation::ERROR_SUCCESS; + use windows_sys::Win32::System::Registry::{HKEY_CURRENT_USER, RRF_RT_REG_SZ, RegGetValueW}; + + let subkey = wide_z(OsStr::new(subkey)); + let mut buf = vec![0u16; 1024]; + let mut bytes = u32::try_from(buf.len() * 2).ok()?; + // SAFETY: `subkey` is a live NUL-terminated UTF-16 string; the default + // value is named by a null pointer; `buf` holds `bytes` writable bytes. + let code = unsafe { + RegGetValueW( + HKEY_CURRENT_USER, + subkey.as_ptr(), + std::ptr::null(), + RRF_RT_REG_SZ, + std::ptr::null_mut(), + buf.as_mut_ptr().cast(), + &mut bytes, + ) + }; + if code != ERROR_SUCCESS { + return None; + } + buf.truncate(bytes as usize / 2); + while buf.last() == Some(&0) { + buf.pop(); + } + Some(std::ffi::OsString::from_wide(&buf)) +} + +fn write_default_string(subkey: &str, value: &OsStr) -> Result<(), String> { + use windows_sys::Win32::Foundation::ERROR_SUCCESS; + use windows_sys::Win32::System::Registry::{HKEY_CURRENT_USER, REG_SZ, RegSetKeyValueW}; + + let subkey_w = wide_z(OsStr::new(subkey)); + let value = wide_z(value); + let bytes = u32::try_from(value.len() * 2).map_err(|_| "value too long".to_string())?; + // SAFETY: both buffers are live NUL-terminated UTF-16 strings, `bytes` + // covers `value` including its terminator, and a null value name is the + // key's default value. `RegSetKeyValueW` creates missing keys. + let code = unsafe { + RegSetKeyValueW( + HKEY_CURRENT_USER, + subkey_w.as_ptr(), + std::ptr::null(), + REG_SZ, + value.as_ptr().cast(), + bytes, + ) + }; + if code == ERROR_SUCCESS { + Ok(()) + } else { + Err(format!( + "write HKCU\\{subkey}: {}", + std::io::Error::from_raw_os_error(code as i32) + )) + } } /// True when `exe` sits in a `cargo` build directory rather than an install. @@ -218,6 +375,7 @@ fn all_users_shortcut_path() -> Option { #[derive(Default)] struct Shortcut { aumid: Option, + toast_activator: Option, target: Option, } @@ -226,6 +384,12 @@ fn wide(s: &OsStr) -> Vec { s.encode_wide().collect() } +fn wide_z(s: &OsStr) -> Vec { + let mut w = wide(s); + w.push(0); + w +} + fn hstring(path: &Path) -> Result { windows::core::HSTRING::from_wide(&wide(path.as_os_str())) .map_err(|e| format!("{}: {e}", path.display())) @@ -241,10 +405,13 @@ fn co_init() { } } -/// Read back the two properties `decide()` cares about. Absent ones come back +/// Read back the properties `decide()` cares about. Absent ones come back /// as `None`; only genuine COM failures are errors. fn read_shortcut(lnk: &Path) -> Result { - use windows::Win32::Storage::EnhancedStorage::PKEY_AppUserModel_ID; + use windows::Win32::Storage::EnhancedStorage::{ + PKEY_AppUserModel_ID, PKEY_AppUserModel_ToastActivatorCLSID, + }; + use windows::Win32::System::Com::StructuredStorage::PropVariantToGUID; use windows::Win32::System::Com::{ CLSCTX_INPROC_SERVER, CoCreateInstance, IPersistFile, STGM_READ, }; @@ -271,6 +438,14 @@ fn read_shortcut(lnk: &Path) -> Result { .and_then(|v| BSTR::try_from(&v).ok()) .map(|s| s.to_string()) .filter(|s| !s.is_empty()); + // VT_CLSID whichever side wrote it. An absent property is VT_EMPTY, + // which `PropVariantToGUID` converts to the nil GUID rather than an + // error, so that is what "not set" looks like here. + let toast_activator = store + .GetValue(&PKEY_AppUserModel_ToastActivatorCLSID) + .ok() + .and_then(|v| PropVariantToGUID(&v).ok()) + .filter(|clsid| *clsid != windows::core::GUID::zeroed()); // SLGP_RAWPATH returns the stored target verbatim; without it the // shell may go looking for a moved file, including over the network. @@ -284,15 +459,22 @@ fn read_shortcut(lnk: &Path) -> Result { }) .filter(|p| !p.as_os_str().is_empty()); - Ok(Shortcut { aumid, target }) + Ok(Shortcut { + aumid, + toast_activator, + target, + }) } } /// Create or overwrite `lnk`, pointing at the running exe and carrying our -/// AUMID. The exe has the icon compiled in (build.rs), which is what the toast +/// AUMID and toast activator. The exe has the icon compiled in (build.rs), which is what the toast /// header shows. fn write_shortcut(lnk: &Path) -> Result<(), String> { - use windows::Win32::Storage::EnhancedStorage::PKEY_AppUserModel_ID; + use windows::Win32::Storage::EnhancedStorage::{ + PKEY_AppUserModel_ID, PKEY_AppUserModel_ToastActivatorCLSID, + }; + use windows::Win32::System::Com::StructuredStorage::InitPropVariantFromCLSID; use windows::Win32::System::Com::{CLSCTX_INPROC_SERVER, CoCreateInstance, IPersistFile}; use windows::Win32::UI::Shell::PropertiesSystem::IPropertyStore; use windows::Win32::UI::Shell::{IShellLinkW, ShellLink}; @@ -314,6 +496,11 @@ fn write_shortcut(lnk: &Path) -> Result<(), String> { store .SetValue(&PKEY_AppUserModel_ID, &PROPVARIANT::from(AUMID)) .map_err(|e| format!("SetValue: {e}"))?; + let clsid = InitPropVariantFromCLSID(&TOAST_ACTIVATOR_CLSID) + .map_err(|e| format!("InitPropVariantFromCLSID: {e}"))?; + store + .SetValue(&PKEY_AppUserModel_ToastActivatorCLSID, &clsid) + .map_err(|e| format!("SetValue: {e}"))?; store.Commit().map_err(|e| format!("Commit: {e}"))?; let persist: IPersistFile = link.cast().map_err(|e| format!("IPersistFile: {e}"))?; persist @@ -341,6 +528,49 @@ mod tests { ); } + #[test] + fn toast_activator_matches_the_installer() { + let iss = std::fs::read_to_string(concat!( + env!("CARGO_MANIFEST_DIR"), + "/.github/scripts/windows-installer.iss" + )) + .expect("read windows-installer.iss"); + let clsid = super::clsid_text(&super::TOAST_ACTIVATOR_CLSID); + let stamp = format!("AppUserModelToastActivatorCLSID: \"{clsid}\""); + assert_eq!( + iss.matches(&stamp).count(), + iss.matches("AppUserModelID:").count(), + "every windows-installer.iss shortcut must set {stamp}" + ); + let cleanup = format!(r"'Software\Classes\CLSID\{{{clsid}}}'"); + assert!( + iss.contains(&cleanup), + "the uninstaller must remove {cleanup}" + ); + } + + #[test] + fn the_clsid_is_spelled_the_registry_way() { + assert_eq!( + super::clsid_text(&super::TOAST_ACTIVATOR_CLSID), + "4E1F6381-E90B-4BED-9156-A9013F718FE6" + ); + assert_eq!( + super::local_server_key(), + r"Software\Classes\CLSID\{4E1F6381-E90B-4BED-9156-A9013F718FE6}\LocalServer32" + ); + } + + /// COM runs this string after appending `-Embedding`; an unquoted path + /// with a space in it would launch `C:\Program` instead. + #[test] + fn the_local_server_command_quotes_the_exe() { + assert_eq!( + super::local_server_command(Path::new(r"C:\Program Files\tty7\tty7-app.exe")), + r#""C:\Program Files\tty7\tty7-app.exe" --toast-activated"# + ); + } + #[test] fn start_menu_shortcut_lives_under_programs() { let Some(lnk) = super::start_menu_shortcut_path() else { @@ -441,6 +671,9 @@ mod tests { written.expect("write shortcut"); let shortcut = read.expect("read attempted").expect("read shortcut"); assert_eq!(shortcut.aumid.as_deref(), Some(super::AUMID)); + // Without it a click on the toast in notification history reaches + // nobody (#1179), and `decide()` would rewrite the shortcut forever. + assert_eq!(shortcut.toast_activator, Some(super::TOAST_ACTIVATOR_CLSID)); let exe = std::env::current_exe().expect("current exe"); let target = shortcut.target.unwrap_or_default(); assert!( @@ -461,7 +694,7 @@ mod tests { fn sends_a_branded_toast() { let decision = match super::decide().expect("decide") { super::Decision::Branded => "already branded".to_string(), - super::Decision::Write(lnk) => format!("writing {}", lnk.display()), + super::Decision::Write { lnk, .. } => format!("writing {}", lnk.display()), super::Decision::Skip(why) => format!("skipping: {why}"), }; println!("decision: {decision}"); diff --git a/src/core/mod.rs b/src/core/mod.rs index b41bbd65..a5020647 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -12,5 +12,7 @@ pub mod keychain; pub mod rate_meter; pub mod session; pub mod ssh_config; +#[cfg(target_os = "windows")] +pub mod toast_activator; pub mod update; pub mod window_state; diff --git a/src/core/toast_activator.rs b/src/core/toast_activator.rs new file mode 100644 index 00000000..61e88502 --- /dev/null +++ b/src/core/toast_activator.rs @@ -0,0 +1,117 @@ +//! The COM class Windows activates when a tty7 toast is clicked. +//! +//! An unpackaged app has two ways to hear about a click. The `Activated` event +//! on a `ToastNotification` only fires while that very object is alive in the +//! process that showed it, and in practice only for the on-screen popup. A +//! toast clicked later from notification history (Action Center) goes the +//! other way: the shell looks up the `ToastActivatorCLSID` on the shortcut +//! that carries our AUMID and calls `INotificationActivationCallback::Activate` +//! on that class — in the running tty7 if it has registered a class object, +//! otherwise by launching the `LocalServer32` command `core::aumid` registers. +//! Without this class, such a click did nothing at all (#1179). +//! +//! The toast's `launch` attribute arrives as `invokedArgs`. It is the same +//! pid-checked identifier macOS uses, so a click on a toast from an earlier +//! process — the only kind a launch made by COM can receive — brings tty7 +//! forward without guessing at a pane. + +use windows::Win32::Foundation::{BOOL, CLASS_E_NOAGGREGATION, E_POINTER}; +use windows::Win32::System::Com::{ + CLSCTX_LOCAL_SERVER, COINIT_MULTITHREADED, CoInitializeEx, CoRegisterClassObject, + IClassFactory, IClassFactory_Impl, REGCLS_MULTIPLEUSE, +}; +use windows::Win32::UI::Notifications::{ + INotificationActivationCallback, INotificationActivationCallback_Impl, + NOTIFICATION_USER_INPUT_DATA, +}; +use windows::core::{GUID, IUnknown, Interface, PCWSTR, implement}; + +/// Register the class object for this process. Idempotent; never blocks. +/// +/// The registration lives on a thread of its own that joins the MTA and then +/// parks for good: COM calls `Activate` on an RPC thread without needing a +/// message loop, and the parked thread keeps the apartment — and with it the +/// registration — alive for the life of the process. +pub(crate) fn start() { + static ONCE: std::sync::Once = std::sync::Once::new(); + ONCE.call_once(|| { + let spawned = std::thread::Builder::new() + .name("tty7-toast-activator".into()) + .spawn(|| { + // SAFETY: plain COM initialization of this new thread. + let _ = unsafe { CoInitializeEx(None, COINIT_MULTITHREADED) }; + let factory: IClassFactory = Factory.into(); + // SAFETY: the CLSID is a live constant and `factory` a valid + // class object; COM holds its own reference from here on. + let registered = unsafe { + CoRegisterClassObject( + &crate::core::aumid::TOAST_ACTIVATOR_CLSID, + &factory, + CLSCTX_LOCAL_SERVER, + REGCLS_MULTIPLEUSE, + ) + }; + if let Err(e) = registered { + log::warn!("toast clicks from notification history will be ignored: {e}"); + return; + } + loop { + std::thread::park(); + } + }); + if let Err(e) = spawned { + log::warn!("failed to start the toast activator thread: {e}"); + } + }); +} + +#[implement(IClassFactory)] +struct Factory; + +impl IClassFactory_Impl for Factory_Impl { + fn CreateInstance( + &self, + outer: Option<&IUnknown>, + iid: *const GUID, + object: *mut *mut core::ffi::c_void, + ) -> windows::core::Result<()> { + if object.is_null() || iid.is_null() { + return Err(E_POINTER.into()); + } + // SAFETY: checked non-null above; COM's contract is that it is writable. + unsafe { *object = std::ptr::null_mut() }; + if outer.is_some() { + return Err(CLASS_E_NOAGGREGATION.into()); + } + let callback: INotificationActivationCallback = Activator.into(); + // SAFETY: `iid` and `object` come straight from COM and were checked. + unsafe { callback.query(iid, object).ok() } + } + + fn LockServer(&self, _lock: BOOL) -> windows::core::Result<()> { + Ok(()) + } +} + +#[implement(INotificationActivationCallback)] +struct Activator; + +impl INotificationActivationCallback_Impl for Activator_Impl { + fn Activate( + &self, + _app_user_model_id: &PCWSTR, + invoked_args: &PCWSTR, + _data: *const NOTIFICATION_USER_INPUT_DATA, + _count: u32, + ) -> windows::core::Result<()> { + let launch = if invoked_args.is_null() { + String::new() + } else { + // SAFETY: a non-null PCWSTR from COM is NUL-terminated and lives + // for the duration of the call. + unsafe { invoked_args.to_string() }.unwrap_or_default() + }; + crate::terminal::toast_activated(&launch); + Ok(()) + } +} diff --git a/src/terminal/mod.rs b/src/terminal/mod.rs index d2155b80..e53f895a 100644 --- a/src/terminal/mod.rs +++ b/src/terminal/mod.rs @@ -33,5 +33,7 @@ mod typeahead; pub mod view; pub(crate) use remote::notify_desktop; +#[cfg(target_os = "windows")] +pub(crate) use remote::toast_activated; pub use remote::{PaneRoute, PaneWorkspace, RemoteTerminal, attach_refused, attach_unanswered}; pub use size::TermSize; diff --git a/src/terminal/remote.rs b/src/terminal/remote.rs index 07390333..ba90a9bc 100644 --- a/src/terminal/remote.rs +++ b/src/terminal/remote.rs @@ -2905,10 +2905,12 @@ fn sanitize_notification_text(s: &str, max_chars: usize) -> String { out } -/// What a click on a macOS notification has to carry back: the pane to reveal. +/// What a click on a notification has to carry back: the pane to reveal. /// -/// It rides in the notification's `identifier`, which is the one field the -/// center hands back verbatim on activation without a dictionary round trip. +/// On macOS it rides in the notification's `identifier`, which is the one +/// field the center hands back verbatim on activation without a dictionary +/// round trip. On Windows it is the toast's `launch` attribute, which comes +/// back as the `invokedArgs` of a COM activation (`core::toast_activator`). /// Shape: `tty7-pane---`. /// /// The pid is what makes a stale identifier fail closed. Notifications outlive @@ -2916,13 +2918,14 @@ fn sanitize_notification_text(s: &str, max_chars: usize) -> String { /// whatever process now owns the bundle id — a relaunched tty7, or a second /// instance running alongside. A leaf id is a gpui entity id, which a fresh /// process hands out again in the same order, so without the pid that click -/// would reveal an unrelated pane. The sequence number keeps two notifications -/// for the same pane apart — the center treats a repeated identifier as -/// "replace the earlier one". -#[cfg(any(target_os = "macos", test))] +/// would reveal an unrelated pane. On Windows that is the *only* kind of click +/// a process COM launched for the purpose can receive. The sequence number +/// keeps two notifications for the same pane apart — the center treats a +/// repeated identifier as "replace the earlier one". +#[cfg(any(target_os = "macos", target_os = "windows", test))] const NOTIFICATION_ID_PREFIX: &str = "tty7-pane-"; -#[cfg(any(target_os = "macos", test))] +#[cfg(any(target_os = "macos", target_os = "windows", test))] fn notification_identifier(leaf_id: u64) -> String { use std::sync::atomic::AtomicU64; static SEQ: AtomicU64 = AtomicU64::new(0); @@ -2931,7 +2934,7 @@ fn notification_identifier(leaf_id: u64) -> String { format!("{NOTIFICATION_ID_PREFIX}{pid}-{leaf_id}-{seq}") } -#[cfg(any(target_os = "macos", test))] +#[cfg(any(target_os = "macos", target_os = "windows", test))] fn leaf_id_in_identifier(identifier: &str) -> Option { let rest = identifier.strip_prefix(NOTIFICATION_ID_PREFIX)?; let (pid, rest) = rest.split_once('-')?; @@ -3066,20 +3069,30 @@ fn try_clickable_notification(title: &str, body: &str, leaf_id: u64) -> bool { let Some(tx) = toast_thread() else { return false; }; - tx.try_send((title.to_string(), body.to_string(), leaf_id)) - .is_ok() + tx.try_send(( + title.to_string(), + body.to_string(), + notification_identifier(leaf_id), + )) + .is_ok() } +/// Title, body, and the `launch` identifier a click hands back. #[cfg(all(target_os = "windows", not(test)))] -type ToastRequest = (String, String, u64); +type ToastRequest = (String, String, String); -/// Toasts are built, shown and *kept* on one dedicated thread. +/// Toasts are built, shown and kept on one dedicated thread. /// -/// A `ToastNotification` has to stay alive for its `Activated` event to reach -/// us, and Windows parks toasts in Action Center long after they leave the -/// screen — a thread per toast sleeping out a fixed window would both leak -/// threads and stop working the moment the timer expired. Keeping them on a -/// single thread also means the WinRT objects never cross a thread boundary. +/// A click reaches us two ways. The one that always works is COM activation +/// (`core::toast_activator`): the shell calls our registered class with the +/// toast's `launch` string, whether the click lands on the popup or on the +/// toast in notification history, and whether or not this process is still +/// the one that sent it. The other is the toast object's own `Activated` +/// event, which fires only for the popup and only while the object is alive +/// here — it is kept as the fallback for a shortcut that predates the toast +/// activator (an all-users install from an older installer, which we cannot +/// rewrite unelevated). Keeping the objects on a single thread also means the +/// WinRT objects never cross a thread boundary. #[cfg(all(target_os = "windows", not(test)))] fn toast_thread() -> Option<&'static std::sync::mpsc::SyncSender> { use std::sync::OnceLock; @@ -3110,13 +3123,14 @@ fn toast_loop(rx: std::sync::mpsc::Receiver) { let _ = CoInitializeEx(None, COINIT_MULTITHREADED); } - // How many past toasts stay clickable. Dropping the oldest is what bounds - // the memory here; there is no useful deadline to expire them on. + // How many recent popups keep their `Activated` fallback. Dropping the + // oldest is what bounds the memory here; the popup is long gone by then, + // and the COM path does not depend on these objects at all. const MAX_LIVE_TOASTS: usize = 32; let mut live: VecDeque = VecDeque::new(); - while let Ok((title, body, leaf_id)) = rx.recv() { - if let Some(toast) = show_windows_toast(&title, &body, leaf_id) { + while let Ok((title, body, launch)) = rx.recv() { + if let Some(toast) = show_windows_toast(&title, &body, launch) { if live.len() == MAX_LIVE_TOASTS { live.pop_front(); } @@ -3125,11 +3139,31 @@ fn toast_loop(rx: std::sync::mpsc::Receiver) { } } +/// The toast's XML. `launch` is what Windows hands back on a click — as the +/// `invokedArgs` of a COM activation, and as the `Arguments` of the in-process +/// `Activated` event. +#[cfg(any(target_os = "windows", test))] +fn toast_xml(title: &str, body: &str, launch: &str) -> String { + format!( + r#" + + + {} + {} + + +"#, + xml_escape(launch), + xml_escape(title), + xml_escape(body) + ) +} + #[cfg(all(target_os = "windows", not(test)))] fn show_windows_toast( title: &str, body: &str, - leaf_id: u64, + launch: String, ) -> Option { use windows::Data::Xml::Dom::XmlDocument; use windows::Foundation::TypedEventHandler; @@ -3138,18 +3172,7 @@ fn show_windows_toast( let app_id = crate::core::aumid::toast_app_id()?; - let xml = format!( - r#" - - - {} - {} - - -"#, - xml_escape(title), - xml_escape(body) - ); + let xml = toast_xml(title, body, &launch); let doc = match XmlDocument::new() { Ok(d) => { @@ -3175,7 +3198,7 @@ fn show_windows_toast( if let Err(e) = toast.Activated(&TypedEventHandler::new( move |_sender: &Option, _args: &Option| { - reveal_pane(leaf_id); + toast_activated(&launch); Ok(()) }, )) { @@ -3198,13 +3221,69 @@ fn show_windows_toast( Some(toast) } +/// What a click on a Windows toast asks for. A pane this process notified +/// about is revealed; anything else — a toast from an earlier tty7, one +/// without a pane, a launch string we do not recognize — still brings tty7 +/// forward, which is what the user clicked a tty7 toast for. +#[cfg(any(target_os = "windows", test))] +fn toast_click_action(launch: &str) -> crate::ui::tray::TrayAction { + use crate::ui::tray::TrayAction; + match leaf_id_in_identifier(launch) { + Some(leaf_id) => TrayAction::RevealPane { leaf_id }, + None => TrayAction::ShowWindow, + } +} + +/// The last few `launch` strings acted on. A click on the popup can arrive +/// twice — through COM and through the toast object's `Activated` event — and +/// must reveal once. Identifiers are unique per toast, so a repeat is that +/// same click; an empty one (a toast without a pane) is never deduplicated. +#[cfg(any(target_os = "windows", test))] +#[derive(Default)] +struct RecentActivations(std::collections::VecDeque); + +#[cfg(any(target_os = "windows", test))] +impl RecentActivations { + const KEEP: usize = 8; + + fn first_time(&mut self, launch: &str) -> bool { + if launch.is_empty() { + return true; + } + if self.0.iter().any(|seen| seen == launch) { + return false; + } + if self.0.len() == Self::KEEP { + self.0.pop_front(); + } + self.0.push_back(launch.to_string()); + true + } +} + +/// A click on a tty7 toast, from whichever path delivered it. Runs on a COM or +/// WinRT callback thread, so it only touches the tray channel — queued there +/// when this process has no dispatch loop yet (a launch COM made to deliver +/// the click). +#[cfg(target_os = "windows")] +#[cfg_attr(test, allow(dead_code))] +pub(crate) fn toast_activated(launch: &str) { + static RECENT: std::sync::Mutex = + std::sync::Mutex::new(RecentActivations(std::collections::VecDeque::new())); + let first = RECENT + .lock() + .map(|mut recent| recent.first_time(launch)) + .unwrap_or(true); + if first { + crate::ui::tray::send_or_queue(toast_click_action(launch)); + } +} + /// Ask the tray dispatch loop to bring `leaf_id` to the front. Runs on whatever /// thread the platform hands the activation to, so it only touches the channel. -#[cfg(any(target_os = "macos", all(target_os = "windows", not(test))))] +#[cfg(target_os = "macos")] fn reveal_pane(leaf_id: u64) { - if let Some(tx) = crate::ui::tray::sender() { - let _ = tx.try_send(crate::ui::tray::TrayAction::RevealPane { leaf_id }); - } + crate::ui::tray::send_or_queue(crate::ui::tray::TrayAction::RevealPane { leaf_id }); } #[cfg(not(any(target_os = "macos", all(target_os = "windows", not(test)))))] @@ -3214,7 +3293,7 @@ fn try_clickable_notification(_title: &str, _body: &str, _leaf_id: u64) -> bool false } -#[cfg(target_os = "windows")] +#[cfg(any(target_os = "windows", test))] fn xml_escape(s: &str) -> String { s.replace('&', "&") .replace('<', "<") @@ -3282,7 +3361,72 @@ mod notification_tests { assert_eq!(sanitize_notification_text("short", 8), "short"); } + /// Pulls the `launch` attribute back out the way the shell would see it. + fn launch_attribute(xml: &str) -> String { + let start = xml + .find(r#"launch=""#) + .expect("toast has a launch attribute") + + 8; + let len = xml[start..].find('"').expect("closed attribute"); + xml[start..start + len].to_string() + } + + /// #1179: a toast clicked from notification history is only reported over + /// COM, as the toast's `launch` string. Without one the pane is lost. + #[test] + fn a_toast_carries_its_pane_in_the_launch_attribute() { + let xml = toast_xml("title", "body", ¬ification_identifier(42)); + assert_eq!( + toast_click_action(&launch_attribute(&xml)), + crate::ui::tray::TrayAction::RevealPane { leaf_id: 42 } + ); + } + + #[test] + fn a_toast_from_another_process_only_shows_the_window() { + let other_pid = std::process::id().wrapping_add(1); + let stale = format!("{NOTIFICATION_ID_PREFIX}{other_pid}-42-0"); + let xml = toast_xml("title", "body", &stale); + assert_eq!( + toast_click_action(&launch_attribute(&xml)), + crate::ui::tray::TrayAction::ShowWindow + ); + assert_eq!( + toast_click_action(""), + crate::ui::tray::TrayAction::ShowWindow + ); + } + + #[test] + fn one_click_delivered_twice_reveals_once() { + let mut recent = RecentActivations::default(); + let a = notification_identifier(1); + let b = notification_identifier(1); + assert!(recent.first_time(&a)); + assert!(!recent.first_time(&a)); + assert!(recent.first_time(&b)); + // Toasts without a pane all share the empty launch string. + assert!(recent.first_time("")); + assert!(recent.first_time("")); + } + + /// The real WinRT XML parser must accept the document and hand the launch + /// string back verbatim. Parses only; shows nothing. #[cfg(target_os = "windows")] + #[test] + fn windows_parses_the_toast_and_keeps_its_launch_string() { + use windows::Data::Xml::Dom::XmlDocument; + let id = notification_identifier(7); + let doc = XmlDocument::new().expect("XmlDocument"); + doc.LoadXml(&toast_xml("a & ", "\"c\" 'd'", &id).into()) + .expect("toast xml parses"); + let launch = doc + .DocumentElement() + .and_then(|root| root.GetAttribute(&"launch".into())) + .expect("launch attribute"); + assert_eq!(launch.to_string(), id); + } + #[test] fn xml_escaping_covers_every_entity_once() { assert_eq!( diff --git a/src/ui/tray/mod.rs b/src/ui/tray/mod.rs index b6e896d0..edb9d582 100644 --- a/src/ui/tray/mod.rs +++ b/src/ui/tray/mod.rs @@ -42,13 +42,37 @@ pub(crate) fn icon_is_up() -> bool { ICON_UP.load(std::sync::atomic::Ordering::Relaxed) } -/// A sender for the current tray dispatch loop, if one is running. -// Only the platform notification callbacks call this. The Windows one is -// compiled out of test builds so unit tests never raise a real toast; the macOS -// one is not, but no test sends a notification there either. -#[cfg_attr(test, allow(dead_code))] -pub(crate) fn sender() -> Option> { - SENDER.lock().ok()?.clone() +/// Actions that arrived before the dispatch loop existed, replayed by `init`. +/// +/// A toast click can reach a process before it has a window: Windows launches +/// tty7 over COM to deliver a click on a toast in notification history, and +/// calls in as soon as the class object is registered — well before the first +/// window, and with it this loop, is up. Bounded so a process whose loop never +/// starts cannot grow it without limit; the oldest click is the one to lose. +static PENDING: Mutex> = + Mutex::new(std::collections::VecDeque::new()); +const MAX_PENDING: usize = 8; + +/// Hand `action` to the tray dispatch loop, or hold it until the loop starts. +// Only the platform notification callbacks call this, and Linux has none. +#[cfg_attr( + any(test, not(any(target_os = "macos", target_os = "windows"))), + allow(dead_code) +)] +pub(crate) fn send_or_queue(action: TrayAction) { + // Checked under the SENDER lock, which `init` also holds while it drains + // the queue, so an action can never land in the queue after the drain. + let Ok(sender) = SENDER.lock() else { return }; + if let Some(tx) = sender.as_ref() { + let _ = tx.try_send(action); + return; + } + if let Ok(mut pending) = PENDING.lock() { + if pending.len() == MAX_PENDING { + pending.pop_front(); + } + pending.push_back(action); + } } #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -407,6 +431,11 @@ pub(crate) fn init(cx: &mut App) { let (tx, rx) = smol::channel::unbounded::(); if let Ok(mut slot) = SENDER.lock() { + if let Ok(mut pending) = PENDING.lock() { + for action in pending.drain(..) { + let _ = tx.try_send(action); + } + } *slot = Some(tx.clone()); }