diff --git a/Cargo.lock b/Cargo.lock index 9044a08fc..5702a53ae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3723,6 +3723,7 @@ dependencies = [ "nix", "objc", "objc2", + "objc2-app-kit", "objc2-foundation", "objc2-security", "objc2-service-management", diff --git a/apps/native/src-tauri/Cargo.toml b/apps/native/src-tauri/Cargo.toml index 3ce9d447f..516491766 100644 --- a/apps/native/src-tauri/Cargo.toml +++ b/apps/native/src-tauri/Cargo.toml @@ -181,8 +181,27 @@ objc2-foundation = { version = "0.3.2", default-features = false, features = [ "std", "NSDictionary", "NSError", + "NSGeometry", + "NSNotification", + "NSObject", + "NSOperation", "NSString", "NSThread", + "NSUserNotification", + "block2", +] } +objc2-app-kit = { version = "0.3", default-features = false, features = [ + "std", + "NSApplication", + "NSButton", + "NSControl", + "NSEvent", + "NSResponder", + "NSScreen", + "NSStatusBarButton", + "NSView", + "NSWindow", + "block2", ] } block2 = "0.6" dispatch2 = "0.3" diff --git a/apps/native/src-tauri/src/attention.rs b/apps/native/src-tauri/src/attention.rs new file mode 100644 index 000000000..e2bc6439f --- /dev/null +++ b/apps/native/src-tauri/src/attention.rs @@ -0,0 +1,1171 @@ +//! Actionable native attention for the transient menu-bar window. +//! +//! On macOS, `NSUserNotificationCenter` has one process-wide delegate. Popover +//! mode therefore routes every notification through this module's owned +//! backend; plugin-backed sends are reserved for control-window mode. Delivery +//! reasserts our delegate immediately before posting so notification clicks +//! keep reopening the popover even if another backend touched the center. + +use std::collections::HashSet; +use std::sync::{LazyLock, Mutex, TryLockError}; + +use tauri::{AppHandle, Manager, Runtime}; + +use crate::shared_types::EvolutionState; + +const NOTIFICATION_ID_PREFIX: &str = "com.darkmatter.nixmac.attention."; +#[cfg(any(target_os = "macos", test))] +const NOTIFICATION_GENERATION_SEPARATOR: &str = ".generation."; +#[cfg(any(target_os = "macos", test))] +static ATTENTION_PROCESS_ID: LazyLock = LazyLock::new(|| uuid::Uuid::new_v4().to_string()); +static ATTENTION_STATE: LazyLock> = + LazyLock::new(|| Mutex::new(AttentionState::default())); + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum AttentionScope { + Work, + Drift, +} + +#[derive(Debug, Default)] +struct ScopeState { + generation: u64, + claimed_notice_ids: HashSet, +} + +#[derive(Debug, Default)] +struct AttentionState { + work: ScopeState, + drift: ScopeState, +} + +impl AttentionState { + fn scope(&self, scope: AttentionScope) -> &ScopeState { + match scope { + AttentionScope::Work => &self.work, + AttentionScope::Drift => &self.drift, + } + } + + fn scope_mut(&mut self, scope: AttentionScope) -> &mut ScopeState { + match scope { + AttentionScope::Work => &mut self.work, + AttentionScope::Drift => &mut self.drift, + } + } + + fn generation(&self, scope: AttentionScope) -> u64 { + self.scope(scope).generation + } + + fn claim_if_current( + &mut self, + scope: AttentionScope, + generation: u64, + id: &str, + eligible: bool, + ) -> bool { + let state = self.scope_mut(scope); + if !eligible || state.generation != generation { + return false; + } + state.claimed_notice_ids.insert(id.to_string()) + } + + fn release_if_current(&mut self, scope: AttentionScope, generation: u64, id: &str) -> bool { + let state = self.scope_mut(scope); + if state.generation != generation { + return false; + } + state.claimed_notice_ids.remove(id) + } + + fn release_current(&mut self, scope: AttentionScope, id: &str) -> bool { + self.scope_mut(scope).claimed_notice_ids.remove(id) + } + + fn clear(&mut self, scope: AttentionScope) -> u64 { + let state = self.scope_mut(scope); + state.generation = state.generation.wrapping_add(1); + state.claimed_notice_ids.clear(); + state.generation + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum AttentionKind { + InputRequired, + ChangesReady, + AnswerReady, + EvolutionPaused, + EvolutionFailed, + BuildSucceeded, + BuildBlocked, + BuildCheckFailed, + BuildFailed, + BuildFinalizationFailed, + RestoreSucceeded, + RestoreFinalizationFailed, + Crash, + Drift, + Test, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct Notice { + id: String, + title: String, + body: String, + request_attention: bool, + scope: AttentionScope, +} + +impl AttentionKind { + fn notice(self, drift: Option<(&str, &str, &str)>) -> Notice { + let (suffix, title, body, request_attention) = match self { + Self::InputRequired => ( + "input-required", + "nixmac needs your input", + "Open nixmac to continue.", + true, + ), + Self::ChangesReady => ( + "changes-ready", + "Your changes are ready to review", + "Open nixmac to review and Build & Test.", + false, + ), + Self::AnswerReady => ( + "answer-ready", + "nixmac has an answer", + "Open nixmac to view the response.", + false, + ), + Self::EvolutionPaused => ( + "evolution-paused", + "nixmac paused the request", + "A safety limit was reached. Open nixmac to review or continue.", + true, + ), + Self::EvolutionFailed => ( + "evolution-failed", + "Your request stopped with an error", + "The request stopped with an error. Open nixmac to review and retry.", + true, + ), + Self::BuildSucceeded => ( + "build-succeeded", + "Build & Test finished", + "Open nixmac to review the result and continue.", + false, + ), + Self::BuildBlocked => ( + "build-blocked", + "Build & Test needs your input", + "Open nixmac to resolve the blocker before continuing.", + true, + ), + Self::BuildCheckFailed => ( + "build-check-failed", + "Configuration check needs attention", + "Open nixmac to review the changes that did not build.", + true, + ), + Self::BuildFailed => ( + "build-failed", + "Build or restore failed", + "Open nixmac to see what failed and the available recovery steps.", + true, + ), + Self::BuildFinalizationFailed => ( + "build-finalization-failed", + "Your changes are active, but nixmac needs attention", + "Open nixmac to review the result and prepare the Save step.", + true, + ), + Self::RestoreSucceeded => ( + "restore-succeeded", + "Restore finished", + "Open nixmac to review the restored state.", + false, + ), + Self::RestoreFinalizationFailed => ( + "restore-finalization-failed", + "Restore finished, but nixmac needs attention", + "Open nixmac to review the restored state and recovery details.", + true, + ), + Self::Crash => ( + "crash", + "nixmac hit an unexpected error", + "Open nixmac to review the error and send feedback.", + true, + ), + Self::Drift => { + let (id, title, body) = drift.expect("drift notice requires its payload"); + return Notice { + id: format!("{NOTIFICATION_ID_PREFIX}drift.{id}"), + title: title.to_string(), + body: body.to_string(), + request_attention: false, + scope: AttentionScope::Drift, + }; + } + Self::Test => ( + "test", + "nixmac notification test", + "Click this notification to reopen nixmac.", + false, + ), + }; + + Notice { + id: format!("{NOTIFICATION_ID_PREFIX}{suffix}"), + title: title.to_string(), + body: body.to_string(), + request_attention, + scope: AttentionScope::Work, + } + } +} + +#[cfg(any(target_os = "macos", test))] +fn owned_notification_scope(identifier: Option<&str>) -> Option { + let suffix = identifier?.strip_prefix(NOTIFICATION_ID_PREFIX)?; + Some(if suffix.starts_with("drift.") { + AttentionScope::Drift + } else { + AttentionScope::Work + }) +} + +#[cfg(any(target_os = "macos", test))] +fn delivery_identifier(id: &str, generation: u64) -> String { + format!( + "{id}{NOTIFICATION_GENERATION_SEPARATOR}{}:{generation}", + *ATTENTION_PROCESS_ID + ) +} + +#[cfg(any(target_os = "macos", test))] +fn should_clear_delivered_notice( + identifier: Option<&str>, + scope: AttentionScope, + generation: u64, +) -> bool { + if owned_notification_scope(identifier) != Some(scope) { + return false; + } + // Clear requests and new deliveries can be queued from different threads. + // A delayed clear must leave notifications from its new generation intact. + // Delivered notices survive app restarts, but the counter does not. A + // different process ID (or a legacy identifier) is always obsolete. + identifier + .and_then(|id| id.rsplit_once(NOTIFICATION_GENERATION_SEPARATOR)) + .and_then(|(_, delivery)| delivery.split_once(':')) + .and_then(|(process_id, generation)| { + generation + .parse::() + .ok() + .map(|generation| (process_id, generation)) + }) + .is_none_or(|(process_id, delivered_generation)| { + process_id != *ATTENTION_PROCESS_ID || delivered_generation < generation + }) +} + +fn completion_kind(state: &EvolutionState, conversational_response: bool) -> AttentionKind { + match state { + EvolutionState::Conversational => AttentionKind::AnswerReady, + EvolutionState::LimitReached => AttentionKind::EvolutionPaused, + EvolutionState::Generated => AttentionKind::ChangesReady, + _ if conversational_response => AttentionKind::AnswerReady, + _ => AttentionKind::ChangesReady, + } +} + +fn should_notify(is_popover: bool, visible: bool, focused: bool, even_if_attended: bool) -> bool { + is_popover && (even_if_attended || !visible || !focused) +} + +fn release_build_check_claim_if_attended( + state: &mut AttentionState, + is_popover: bool, + visible: bool, + focused: bool, +) -> bool { + if !is_popover || !visible || !focused { + return false; + } + state.release_current( + AttentionScope::Work, + &AttentionKind::BuildCheckFailed.notice(None).id, + ) +} + +fn with_attention_state(f: impl FnOnce(&mut AttentionState) -> T) -> T { + let mut state = match ATTENTION_STATE.lock() { + Ok(guard) => guard, + Err(poisoned) => poisoned.into_inner(), + }; + f(&mut state) +} + +fn attention_generation(scope: AttentionScope) -> u64 { + with_attention_state(|state| state.generation(scope)) +} + +fn claim_notice(scope: AttentionScope, generation: u64, id: &str, eligible: bool) -> bool { + with_attention_state(|state| state.claim_if_current(scope, generation, id, eligible)) +} + +fn clear_attention_state(scope: AttentionScope) -> u64 { + with_attention_state(|state| state.clear(scope)) +} + +fn release_claim(scope: AttentionScope, generation: u64, id: &str) { + with_attention_state(|state| state.release_if_current(scope, generation, id)); +} + +fn send( + app: &AppHandle, + notice: Notice, + even_if_attended: bool, +) -> Result { + let generation = attention_generation(notice.scope); + send_in_generation(app, notice, even_if_attended, generation) +} + +fn send_in_generation( + app: &AppHandle, + notice: Notice, + even_if_attended: bool, + generation: u64, +) -> Result { + let scope = notice.scope; + + #[cfg(target_os = "macos")] + { + let app_for_delivery = app.clone(); + if let Err(error) = app.run_on_main_thread(move || { + let is_popover = crate::main_window::active(&app_for_delivery).is_popover(); + let (visible, focused) = app_for_delivery + .get_webview_window("main") + .map(|window| { + ( + window.is_visible().unwrap_or(false), + window.is_focused().unwrap_or(false), + ) + }) + .unwrap_or((false, false)); + let eligible = should_notify(is_popover, visible, focused, even_if_attended); + if !claim_notice(scope, generation, ¬ice.id, eligible) { + return; + } + + if notice.request_attention + && let Some(window) = app_for_delivery.get_webview_window("main") + && let Err(error) = + window.request_user_attention(Some(tauri::UserAttentionType::Informational)) + { + log::warn!("Failed to request user attention: {error}"); + } + + let delivery_id = notice.id.clone(); + if let Err(error) = macos::deliver(notice, generation) { + release_claim(scope, generation, &delivery_id); + log::warn!("Failed to send attention notification: {error}"); + } + }) { + return Err(error.to_string()); + } + } + + #[cfg(not(target_os = "macos"))] + { + use tauri_plugin_notification::NotificationExt; + + let is_popover = crate::main_window::active(app).is_popover(); + let (visible, focused) = app + .get_webview_window("main") + .map(|window| { + ( + window.is_visible().unwrap_or(false), + window.is_focused().unwrap_or(false), + ) + }) + .unwrap_or((false, false)); + let eligible = should_notify(is_popover, visible, focused, even_if_attended); + if !claim_notice(scope, generation, ¬ice.id, eligible) { + return Ok(false); + } + + if notice.request_attention + && let Some(window) = app.get_webview_window("main") + && let Err(error) = + window.request_user_attention(Some(tauri::UserAttentionType::Informational)) + { + log::warn!("Failed to request user attention: {error}"); + } + + if let Err(error) = app + .notification() + .builder() + .title(notice.title) + .body(notice.body) + .show() + { + release_claim(scope, generation, ¬ice.id); + return Err(error.to_string()); + } + } + + Ok(true) +} + +pub(crate) fn install(app: &AppHandle) -> Result<(), String> { + if !crate::main_window::active(app).is_popover() { + return Ok(()); + } + install_backend() +} + +pub(crate) fn plugin_notification_backend_allowed_for_mode(is_popover: bool) -> bool { + #[cfg(target_os = "macos")] + { + !is_popover + } + #[cfg(not(target_os = "macos"))] + { + let _ = is_popover; + true + } +} + +/// Whether a caller may use `tauri_plugin_notification` without competing +/// with the popover's process-wide macOS notification delegate. +pub(crate) fn plugin_notification_backend_allowed(app: &AppHandle) -> bool { + plugin_notification_backend_allowed_for_mode(crate::main_window::active(app).is_popover()) +} + +#[cfg(target_os = "macos")] +fn install_backend() -> Result<(), String> { + macos::install() +} + +#[cfg(not(target_os = "macos"))] +fn install_backend() -> Result<(), String> { + Ok(()) +} + +pub(crate) fn shutdown() { + #[cfg(target_os = "macos")] + macos::shutdown(); +} + +fn clear_scope(_app: &AppHandle, scope: AttentionScope) { + let _generation = clear_attention_state(scope); + #[cfg(target_os = "macos")] + if let Err(error) = _app.run_on_main_thread(move || macos::clear_delivered(scope, _generation)) + { + log::debug!("Could not clear delivered attention notifications: {error}"); + } +} + +pub(crate) fn clear_work(app: &AppHandle) { + clear_scope(app, AttentionScope::Work); +} + +pub(crate) fn clear_drift(app: &AppHandle) { + clear_scope(app, AttentionScope::Drift); +} + +pub(crate) fn input_required(app: &AppHandle) { + if let Err(error) = send(app, AttentionKind::InputRequired.notice(None), false) { + log::warn!("Failed to send input-required attention: {error}"); + } +} + +pub(crate) fn evolution_completed( + app: &AppHandle, + state: &EvolutionState, + conversational_response: bool, +) { + if let Err(error) = send( + app, + completion_kind(state, conversational_response).notice(None), + false, + ) { + log::warn!("Failed to send evolution-complete attention: {error}"); + } +} + +pub(crate) fn evolution_failed(app: &AppHandle) { + if let Err(error) = send(app, AttentionKind::EvolutionFailed.notice(None), false) { + log::warn!("Failed to send evolution-failed attention: {error}"); + } +} + +pub(crate) fn build_succeeded(app: &AppHandle) { + if let Err(error) = send(app, AttentionKind::BuildSucceeded.notice(None), false) { + log::warn!("Failed to send build-succeeded attention: {error}"); + } +} + +pub(crate) fn build_failed(app: &AppHandle) { + if let Err(error) = send(app, AttentionKind::BuildFailed.notice(None), false) { + log::warn!("Failed to send build-failed attention: {error}"); + } +} + +pub(crate) fn build_blocked(app: &AppHandle) { + if let Err(error) = send(app, AttentionKind::BuildBlocked.notice(None), false) { + log::warn!("Failed to send build-blocked attention: {error}"); + } +} + +pub(crate) fn build_check_failed(app: &AppHandle) { + if let Err(error) = send(app, AttentionKind::BuildCheckFailed.notice(None), false) { + log::warn!("Failed to send build-check attention: {error}"); + } +} + +pub(crate) fn begin_build_check(app: &AppHandle) { + let is_popover = crate::main_window::active(app).is_popover(); + let (visible, focused) = app + .get_webview_window("main") + .map(|window| { + ( + window.is_visible().unwrap_or(false), + window.is_focused().unwrap_or(false), + ) + }) + .unwrap_or((false, false)); + with_attention_state(|state| { + release_build_check_claim_if_attended(state, is_popover, visible, focused) + }); +} + +pub(crate) fn build_finalization_failed(app: &AppHandle) { + if let Err(error) = send( + app, + AttentionKind::BuildFinalizationFailed.notice(None), + false, + ) { + log::warn!("Failed to send build-finalization attention: {error}"); + } +} + +pub(crate) fn restore_finished(app: &AppHandle) { + if let Err(error) = send(app, AttentionKind::RestoreSucceeded.notice(None), false) { + log::warn!("Failed to send restore-finished attention: {error}"); + } +} + +pub(crate) fn restore_finalization_failed(app: &AppHandle) { + if let Err(error) = send( + app, + AttentionKind::RestoreFinalizationFailed.notice(None), + false, + ) { + log::warn!("Failed to send restore-finalization attention: {error}"); + } +} + +fn try_crash_generation(state: &Mutex) -> Option { + let state = match state.try_lock() { + Ok(state) => state, + Err(TryLockError::Poisoned(error)) => error.into_inner(), + Err(TryLockError::WouldBlock) => return None, + }; + Some(state.generation(AttentionScope::Work)) +} + +pub(crate) fn crash(app: &AppHandle) { + // A panic may originate inside an attention-state mutation. Never wait on + // its lock in the hook: unwinding must be able to release that same lock. + let Some(generation) = try_crash_generation(&ATTENTION_STATE) else { + return; + }; + if let Err(error) = + send_in_generation(app, AttentionKind::Crash.notice(None), false, generation) + { + log::warn!("Failed to send crash attention: {error}"); + } +} + +pub(crate) fn drift(app: &AppHandle, id: &str, title: &str, body: &str) { + if let Err(error) = send( + app, + AttentionKind::Drift.notice(Some((id, title, body))), + false, + ) { + log::warn!("Failed to send drift attention: {error}"); + } +} + +pub(crate) fn test(app: &AppHandle) { + clear_work(app); + if let Err(error) = send(app, AttentionKind::Test.notice(None), true) { + log::warn!("Failed to send test attention: {error}"); + } +} + +/// The popover's owned macOS notification backend. +/// +/// It uses `NSUserNotificationCenter`, deprecated since macOS 11, on purpose: +/// `tauri-plugin-notification` posts through the same center on macOS (via +/// `notify-rust` and `mac-notification-sys`) and exposes no click callback, +/// so it cannot reopen the popover. Moving to `UNUserNotificationCenter` +/// needs its own authorization request, block-based delegate callbacks and a +/// plan for the plugin's sends in control-window mode, so it is tracked as +/// its own change (ENG-1449). +/// +/// `#[allow(deprecated)]` is scoped to each item that touches the deprecated +/// types, never the whole module, so any other deprecation here still warns. +#[cfg(target_os = "macos")] +mod macos { + use std::cell::RefCell; + + use objc2::runtime::ProtocolObject; + use objc2::{MainThreadOnly, define_class, msg_send, rc::Retained}; + use objc2_foundation::{MainThreadMarker, NSObject, NSObjectProtocol, NSString}; + #[allow(deprecated)] + use objc2_foundation::{ + NSUserNotification, NSUserNotificationCenter, NSUserNotificationCenterDelegate, + }; + + use super::{ + AttentionScope, Notice, delivery_identifier, owned_notification_scope, + should_clear_delivered_notice, + }; + + #[derive(Debug, Clone, Copy, PartialEq, Eq)] + enum ActivationAction { + Ignore, + OpenMain, + } + + fn activation_action(identifier: Option<&str>) -> ActivationAction { + if owned_notification_scope(identifier).is_some() { + ActivationAction::OpenMain + } else { + ActivationAction::Ignore + } + } + + define_class!( + // SAFETY: NSObject has no subclassing requirements and this class has no Drop impl. + #[unsafe(super = NSObject)] + #[thread_kind = MainThreadOnly] + #[ivars = ()] + struct AttentionDelegate; + + // SAFETY: These delegate protocols add no invariants beyond their method signatures. + unsafe impl NSObjectProtocol for AttentionDelegate {} + + #[allow(deprecated)] + unsafe impl NSUserNotificationCenterDelegate for AttentionDelegate { + #[unsafe(method(userNotificationCenter:didActivateNotification:))] + fn did_activate( + &self, + center: &NSUserNotificationCenter, + notification: &NSUserNotification, + ) { + let identifier = notification.identifier().map(|value| value.to_string()); + if activation_action(identifier.as_deref()) != ActivationAction::OpenMain { + return; + } + + if let Some(app) = crate::APP_HANDLE.get() { + let mode = crate::main_window::active(app); + match crate::main_window::show(app, mode) { + Ok(()) => center.removeDeliveredNotification(notification), + Err(error) => { + log::warn!("Failed to open nixmac from a notification: {error}") + } + } + } + } + + #[unsafe(method(userNotificationCenter:shouldPresentNotification:))] + fn should_present( + &self, + _center: &NSUserNotificationCenter, + notification: &NSUserNotification, + ) -> bool { + let identifier = notification.identifier().map(|value| value.to_string()); + activation_action(identifier.as_deref()) == ActivationAction::OpenMain + } + } + ); + + impl AttentionDelegate { + fn new(mtm: MainThreadMarker) -> Retained { + let this = Self::alloc(mtm).set_ivars(()); + // SAFETY: NSObject's init signature is correct for this ivar-free subclass. + unsafe { msg_send![super(this), init] } + } + } + + #[allow(deprecated)] + struct Installed { + center: Retained, + delegate: Retained, + } + + impl Installed { + #[allow(deprecated)] + fn reassert_delegate(&self) { + // SAFETY: NSUserNotificationCenter keeps an unretained pointer; `delegate` is retained + // alongside the center in main-thread storage for the entire installation lifetime. + unsafe { + self.center + .setDelegate(Some(ProtocolObject::from_ref(&*self.delegate))); + } + } + } + + impl Drop for Installed { + #[allow(deprecated)] + fn drop(&mut self) { + // SAFETY: Only clear the unretained pointer when it still points at our retained + // delegate; another notification backend may have replaced it in the meantime. + let current = unsafe { self.center.delegate() }; + let ours: &ProtocolObject = + ProtocolObject::from_ref(&*self.delegate); + if current + .as_deref() + .is_some_and(|delegate| std::ptr::eq(delegate, ours)) + { + unsafe { self.center.setDelegate(None) }; + } + } + } + + thread_local! { + static INSTALLED: RefCell> = const { RefCell::new(None) }; + } + + #[allow(deprecated)] + pub(super) fn install() -> Result<(), String> { + let mtm = MainThreadMarker::new().ok_or_else(|| { + "Attention notifications must be installed on the main thread".to_string() + })?; + INSTALLED.with(|slot| { + let mut installed = slot.borrow_mut(); + if installed.is_none() { + *installed = Some(Installed { + center: NSUserNotificationCenter::defaultUserNotificationCenter(), + delegate: AttentionDelegate::new(mtm), + }); + } + installed + .as_ref() + .expect("attention delegate was just installed") + .reassert_delegate(); + }); + Ok(()) + } + + #[allow(deprecated)] + pub(super) fn deliver(notice: Notice, generation: u64) -> Result<(), String> { + install()?; + INSTALLED.with(|slot| { + let installed = slot.borrow(); + let installed = installed + .as_ref() + .ok_or_else(|| "Attention notification delegate is unavailable".to_string())?; + installed.reassert_delegate(); + + let notification = NSUserNotification::new(); + notification.setIdentifier(Some(&NSString::from_str(&delivery_identifier( + ¬ice.id, generation, + )))); + notification.setTitle(Some(&NSString::from_str(¬ice.title))); + notification.setInformativeText(Some(&NSString::from_str(¬ice.body))); + installed.center.deliverNotification(¬ification); + Ok(()) + }) + } + + #[allow(deprecated)] + pub(super) fn clear_delivered(scope: AttentionScope, generation: u64) { + INSTALLED.with(|slot| { + if let Some(installed) = slot.borrow().as_ref() { + let delivered = installed.center.deliveredNotifications(); + for notification in delivered.iter() { + let identifier = notification.identifier().map(|value| value.to_string()); + if should_clear_delivered_notice(identifier.as_deref(), scope, generation) { + installed.center.removeDeliveredNotification(¬ification); + } + } + } + }); + } + + pub(super) fn shutdown() { + INSTALLED.with(|slot| { + slot.borrow_mut().take(); + }); + } + + #[cfg(test)] + mod tests { + use super::*; + + #[test] + fn only_owned_attention_notifications_open_the_main_window() { + assert_eq!( + activation_action(Some("com.darkmatter.nixmac.attention.question")), + ActivationAction::OpenMain + ); + assert_eq!( + activation_action(Some("com.darkmatter.nixmac.other")), + ActivationAction::Ignore + ); + assert_eq!(activation_action(None), ActivationAction::Ignore); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn completion_attention_distinguishes_review_answer_and_pause() { + assert_eq!( + completion_kind(&EvolutionState::Generated, false), + AttentionKind::ChangesReady + ); + assert_eq!( + completion_kind(&EvolutionState::Conversational, true), + AttentionKind::AnswerReady + ); + assert_eq!( + completion_kind(&EvolutionState::LimitReached, false), + AttentionKind::EvolutionPaused + ); + } + + #[test] + fn popover_attention_only_fires_when_away() { + assert!(!should_notify(true, true, true, false)); + assert!(should_notify(true, true, false, false)); + assert!(should_notify(true, false, true, false)); + assert!(should_notify(true, true, true, true)); + assert!(!should_notify(false, false, false, true)); + } + + #[test] + fn notice_copy_is_actionable_and_privacy_safe() { + let input = AttentionKind::InputRequired.notice(None); + assert_eq!(input.title, "nixmac needs your input"); + assert_eq!(input.body, "Open nixmac to continue."); + assert!(input.id.starts_with(NOTIFICATION_ID_PREFIX)); + + let failure = AttentionKind::EvolutionFailed.notice(None); + assert_eq!(failure.title, "Your request stopped with an error"); + } + + #[test] + fn build_outcomes_distinguish_success_failure_finalization_and_restore() { + let success = AttentionKind::BuildSucceeded.notice(None); + let failure = AttentionKind::BuildFailed.notice(None); + let finalization = AttentionKind::BuildFinalizationFailed.notice(None); + let restore = AttentionKind::RestoreSucceeded.notice(None); + let restore_finalization = AttentionKind::RestoreFinalizationFailed.notice(None); + + assert_eq!(success.title, "Build & Test finished"); + assert!(!success.request_attention); + assert_eq!(failure.title, "Build or restore failed"); + assert_eq!( + failure.body, + "Open nixmac to see what failed and the available recovery steps." + ); + assert!(failure.request_attention); + assert!(finalization.title.contains("changes are active")); + assert_eq!(restore.title, "Restore finished"); + assert!(restore_finalization.title.contains("Restore finished")); + } + + #[test] + fn panic_attention_does_not_wait_on_an_already_held_state_lock() { + let state = Mutex::new(AttentionState::default()); + let held = state.lock().unwrap(); + assert_eq!(try_crash_generation(&state), None); + drop(held); + assert_eq!(try_crash_generation(&state), Some(0)); + } + + #[test] + fn attended_notice_does_not_consume_its_claim() { + let mut state = AttentionState::default(); + let generation = state.generation(AttentionScope::Work); + + assert!(!state.claim_if_current( + AttentionScope::Work, + generation, + "same", + should_notify(true, true, true, false), + )); + assert!(state.claim_if_current( + AttentionScope::Work, + generation, + "same", + should_notify(true, false, false, false), + )); + assert!(!state.claim_if_current(AttentionScope::Work, generation, "same", true)); + } + + #[test] + fn stale_generation_cannot_claim_after_clear() { + let mut state = AttentionState::default(); + let stale_generation = state.generation(AttentionScope::Work); + + state.clear(AttentionScope::Work); + + assert!(!state.claim_if_current(AttentionScope::Work, stale_generation, "stale", true)); + assert!(state.claim_if_current( + AttentionScope::Work, + state.generation(AttentionScope::Work), + "fresh", + true + )); + } + + #[test] + fn delayed_clear_preserves_deliveries_from_the_new_generation() { + let mut state = AttentionState::default(); + let id = AttentionKind::BuildSucceeded.notice(None).id; + let old_id = delivery_identifier(&id, state.generation(AttentionScope::Work)); + let clearing_generation = state.clear(AttentionScope::Work); + let fresh_id = delivery_identifier(&id, state.generation(AttentionScope::Work)); + + // Another thread queued a fresh delivery before this clear's Cocoa + // callback reached the main thread. It must remain actionable. + assert!(should_clear_delivered_notice( + Some(&old_id), + AttentionScope::Work, + clearing_generation, + )); + assert!(!should_clear_delivered_notice( + Some(&fresh_id), + AttentionScope::Work, + clearing_generation, + )); + assert!(should_clear_delivered_notice( + Some(&fresh_id), + AttentionScope::Work, + state.clear(AttentionScope::Work), + )); + } + + #[test] + fn clearing_delivered_notices_preserves_other_scopes_and_removes_legacy_ids() { + let work = AttentionKind::BuildSucceeded.notice(None).id; + let drift = AttentionKind::Drift + .notice(Some(("external-build:1", "Drift", "Review"))) + .id; + assert!(should_clear_delivered_notice( + Some(&work), + AttentionScope::Work, + 1 + )); + assert!(!should_clear_delivered_notice( + Some(&drift), + AttentionScope::Work, + 1 + )); + assert!(!should_clear_delivered_notice( + None, + AttentionScope::Work, + 1 + )); + assert!(should_clear_delivered_notice( + Some(&format!( + "{work}{NOTIFICATION_GENERATION_SEPARATOR}previous-process:100" + )), + AttentionScope::Work, + 1, + )); + } + + #[test] + fn clear_resets_claims_for_the_next_generation() { + let mut state = AttentionState::default(); + let first_generation = state.generation(AttentionScope::Work); + assert!(state.claim_if_current(AttentionScope::Work, first_generation, "same", true)); + assert!(!state.claim_if_current(AttentionScope::Work, first_generation, "same", true)); + + state.clear(AttentionScope::Work); + + let next_generation = state.generation(AttentionScope::Work); + assert_ne!(next_generation, first_generation); + assert!(state.claim_if_current(AttentionScope::Work, next_generation, "same", true)); + } + + #[test] + fn failed_delivery_releases_only_its_claim_for_retry() { + let mut state = AttentionState::default(); + let generation = state.generation(AttentionScope::Work); + assert!(state.claim_if_current(AttentionScope::Work, generation, "failed", true)); + assert!(state.claim_if_current(AttentionScope::Work, generation, "unrelated", true)); + + assert!(state.release_if_current(AttentionScope::Work, generation, "failed")); + + assert!(state.claim_if_current(AttentionScope::Work, generation, "failed", true)); + assert!(!state.claim_if_current(AttentionScope::Work, generation, "unrelated", true)); + } + + #[test] + fn failed_delivery_releases_only_the_matching_scope() { + let mut state = AttentionState::default(); + let work_generation = state.generation(AttentionScope::Work); + let drift_generation = state.generation(AttentionScope::Drift); + assert!(state.claim_if_current(AttentionScope::Work, work_generation, "same", true)); + assert!(state.claim_if_current(AttentionScope::Drift, drift_generation, "same", true)); + + assert!(state.release_if_current(AttentionScope::Drift, drift_generation, "same")); + + assert!(state.claim_if_current(AttentionScope::Drift, drift_generation, "same", true)); + assert!(!state.claim_if_current(AttentionScope::Work, work_generation, "same", true)); + } + + #[test] + fn stale_delivery_failure_does_not_release_a_new_generation_claim() { + let mut state = AttentionState::default(); + let stale_generation = state.generation(AttentionScope::Work); + assert!(state.claim_if_current(AttentionScope::Work, stale_generation, "same", true)); + state.clear(AttentionScope::Work); + let current_generation = state.generation(AttentionScope::Work); + assert!(state.claim_if_current(AttentionScope::Work, current_generation, "same", true)); + + assert!(!state.release_if_current(AttentionScope::Work, stale_generation, "same")); + + assert!(!state.claim_if_current(AttentionScope::Work, current_generation, "same", true)); + } + + #[test] + fn attended_build_check_release_is_targeted() { + let mut state = AttentionState::default(); + let generation = state.generation(AttentionScope::Work); + let build_check_id = AttentionKind::BuildCheckFailed.notice(None).id; + let other_id = AttentionKind::BuildFailed.notice(None).id; + assert!(state.claim_if_current(AttentionScope::Work, generation, &build_check_id, true)); + assert!(state.claim_if_current(AttentionScope::Work, generation, &other_id, true)); + + assert!(release_build_check_claim_if_attended( + &mut state, true, true, true, + )); + + assert!(state.claim_if_current(AttentionScope::Work, generation, &build_check_id, true)); + assert!(!state.claim_if_current(AttentionScope::Work, generation, &other_id, true)); + } + + #[test] + fn hidden_or_unfocused_build_checks_keep_the_failure_claim() { + for (is_popover, visible, focused) in [ + (true, false, false), + (true, false, true), + (true, true, false), + (false, true, true), + ] { + let mut state = AttentionState::default(); + let generation = state.generation(AttentionScope::Work); + let build_check_id = AttentionKind::BuildCheckFailed.notice(None).id; + assert!(state.claim_if_current( + AttentionScope::Work, + generation, + &build_check_id, + true + )); + + assert!(!release_build_check_claim_if_attended( + &mut state, is_popover, visible, focused, + )); + assert!(!state.claim_if_current( + AttentionScope::Work, + generation, + &build_check_id, + true + )); + } + } + + #[test] + fn developer_test_notice_can_notify_while_attended() { + let mut state = AttentionState::default(); + let generation = state.generation(AttentionScope::Work); + + assert!(state.claim_if_current( + AttentionScope::Work, + generation, + "test", + should_notify(true, true, true, true), + )); + assert!(!state.claim_if_current( + AttentionScope::Work, + generation, + "control-mode-test", + should_notify(false, false, false, true), + )); + } + + #[test] + fn clearing_work_releases_work_without_consuming_drift() { + let mut state = AttentionState::default(); + let work_generation = state.generation(AttentionScope::Work); + let drift_generation = state.generation(AttentionScope::Drift); + assert!(state.claim_if_current(AttentionScope::Work, work_generation, "same-work", true)); + assert!(state.claim_if_current( + AttentionScope::Drift, + drift_generation, + "same-drift", + true + )); + + state.clear(AttentionScope::Work); + + assert!(state.claim_if_current( + AttentionScope::Work, + state.generation(AttentionScope::Work), + "same-work", + true + )); + assert!(!state.claim_if_current( + AttentionScope::Drift, + drift_generation, + "same-drift", + true + )); + } + + #[test] + fn delivered_notice_scopes_only_match_owned_identifiers() { + assert_eq!( + owned_notification_scope(Some("com.darkmatter.nixmac.attention.build-failed")), + Some(AttentionScope::Work) + ); + assert_eq!( + owned_notification_scope(Some( + "com.darkmatter.nixmac.attention.drift.config-drift:abc123" + )), + Some(AttentionScope::Drift) + ); + assert_eq!( + owned_notification_scope(Some("com.darkmatter.nixmac.someone-elses-notice")), + None + ); + } + + #[test] + fn macos_popover_reserves_the_owned_notification_backend() { + #[cfg(target_os = "macos")] + { + assert!(!plugin_notification_backend_allowed_for_mode(true)); + assert!(plugin_notification_backend_allowed_for_mode(false)); + } + } +} diff --git a/apps/native/src-tauri/src/commands/apply.rs b/apps/native/src-tauri/src/commands/apply.rs index 40c5fcdfc..6e1b9dc94 100644 --- a/apps/native/src-tauri/src/commands/apply.rs +++ b/apps/native/src-tauri/src/commands/apply.rs @@ -7,25 +7,38 @@ use tauri::AppHandle; pub async fn check_etc_clobber( app: AppHandle, ) -> Result { + crate::attention::clear_work(&app); let (host_attr, config_dir) = get_hostname_and_config_dir(&app, "darwin_check_etc_clobber")?; - crate::system::etc_preflight::check_etc_clobber(&config_dir, &host_attr) - .map_err(|e| capture_err("darwin_check_etc_clobber", e)) + let result = crate::system::etc_preflight::check_etc_clobber(&config_dir, &host_attr) + .map_err(|e| capture_err("darwin_check_etc_clobber", e))?; + if !result.ok { + crate::attention::build_blocked(&app); + } + Ok(result) } pub async fn check_app_management( app: AppHandle, ) -> Result { + crate::attention::clear_work(&app); let (host_attr, config_dir) = get_hostname_and_config_dir(&app, "darwin_check_app_management")?; - crate::system::app_management_preflight::check_app_management(&config_dir, &host_attr) - .map_err(|e| capture_err("darwin_check_app_management", e)) + let result = + crate::system::app_management_preflight::check_app_management(&config_dir, &host_attr) + .map_err(|e| capture_err("darwin_check_app_management", e))?; + if !result.ok { + crate::attention::build_blocked(&app); + } + Ok(result) } pub async fn start_apply_stream( app: AppHandle, host_override: Option, ) -> Result { - let dir = store::ensure_config_dir_exists(&app) - .map_err(|e| capture_err("darwin_apply_stream_start", e))?; + let dir = store::ensure_config_dir_exists(&app).map_err(|error| { + crate::attention::build_blocked(&app); + capture_err("darwin_apply_stream_start", error) + })?; let stored_attr = nix::determine_host_attr(&app); let discovered_hosts = nix::list_darwin_hosts(&dir).ok(); @@ -45,11 +58,14 @@ pub async fn start_apply_stream( (hosts.len() == 1).then(|| hosts[0].clone()) }) .ok_or_else(|| { + crate::attention::build_blocked(&app); "Host attribute not found. Set a host in Settings or ensure your flake defines exactly one darwinConfiguration.".to_string() })?; - rebuild::apply_stream(&app, &dir, &host) - .map_err(|e| capture_err("darwin_apply_stream_start", e))?; + rebuild::apply_stream(&app, &dir, &host).map_err(|error| { + crate::attention::build_blocked(&app); + capture_err("darwin_apply_stream_start", error) + })?; Ok(shared_types::OkResult::yes()) } @@ -63,9 +79,11 @@ pub async fn activate_store_path( } pub async fn run_finalize_apply(app: AppHandle) -> Result<(), String> { - crate::rebuild::finalize_apply(&app) - .await - .map_err(|e| capture_err("finalize_apply", e)) + if let Err(error) = crate::rebuild::finalize_apply(&app).await { + crate::attention::build_finalization_failed(&app); + return Err(capture_err("finalize_apply", error)); + } + Ok(()) } pub async fn run_finalize_rollback( @@ -73,9 +91,11 @@ pub async fn run_finalize_rollback( store_path: Option, changeset_id: Option, ) -> Result<(), String> { - crate::rebuild::finalize_rollback(&app, store_path, changeset_id) - .await - .map_err(|e| capture_err("finalize_rollback", e)) + if let Err(error) = crate::rebuild::finalize_rollback(&app, store_path, changeset_id).await { + crate::attention::restore_finalization_failed(&app); + return Err(capture_err("finalize_rollback", error)); + } + Ok(()) } pub async fn fetch_rebuild_status(app: AppHandle) -> Result { diff --git a/apps/native/src-tauri/src/commands/debug.rs b/apps/native/src-tauri/src/commands/debug.rs index 2f40c2364..7935334fe 100644 --- a/apps/native/src-tauri/src/commands/debug.rs +++ b/apps/native/src-tauri/src/commands/debug.rs @@ -65,7 +65,11 @@ pub async fn developer_send_test_notification(app: AppHandle) -> Result<(), Stri return Err("Developer mode is required to send test notifications".to_string()); } - crate::state::drift_notifications::maybe_notify(None, true); + if crate::main_window::active(&app).is_popover() { + crate::attention::test(&app); + } else { + crate::state::drift_notifications::maybe_notify(None, true); + } Ok(()) } diff --git a/apps/native/src-tauri/src/commands/evolve.rs b/apps/native/src-tauri/src/commands/evolve.rs index e65e4b7f1..7df62c3da 100644 --- a/apps/native/src-tauri/src/commands/evolve.rs +++ b/apps/native/src-tauri/src/commands/evolve.rs @@ -9,6 +9,8 @@ use tauri::AppHandle; /// point — the app may exit right after this returns) and the session path /// cleared exactly once, so no exit path can forget them. pub async fn run_evolve(app: AppHandle, description: String) -> Result<(), String> { + let _active_evolution = evolve::session_control::ActiveEvolutionGuard::begin(); + crate::attention::clear_work(&app); let result = run_evolve_session(app, description).await; crate::state::session_log::flush_ordered().await; crate::state::session_log::set_session_path(None); @@ -58,6 +60,7 @@ async fn run_evolve_session(app: AppHandle, description: String) -> Result<(), S failure.telemetry.iterations, failure.telemetry.build_attempts ); + crate::attention::evolution_failed(&app); return Err(capture_err("darwin_evolve", failure.error)); } }; diff --git a/apps/native/src-tauri/src/commands/peek.rs b/apps/native/src-tauri/src/commands/peek.rs index c63487730..96c2ad552 100644 --- a/apps/native/src-tauri/src/commands/peek.rs +++ b/apps/native/src-tauri/src/commands/peek.rs @@ -1,5 +1,5 @@ use super::helpers::capture_err; -use crate::{peek, shared_types}; +use crate::{main_window, peek, shared_types}; use tauri::AppHandle; pub async fn show_preview_indicator(app: AppHandle) -> Result { @@ -32,7 +32,12 @@ pub async fn fetch_preview_indicator_state() -> Result Result { - peek::show_main_window(&app).map_err(|e| capture_err("show_main_window", e))?; + let mode = main_window::active(&app); + if mode.is_popover() { + main_window::show(&app, mode).map_err(|e| capture_err("show_main_window", e))?; + } else { + peek::show_main_window(&app).map_err(|e| capture_err("show_main_window", e))?; + } Ok(shared_types::OkResult::yes()) } diff --git a/apps/native/src-tauri/src/commands/rollback.rs b/apps/native/src-tauri/src/commands/rollback.rs index 6e5cec13a..c47b511ce 100644 --- a/apps/native/src-tauri/src/commands/rollback.rs +++ b/apps/native/src-tauri/src/commands/rollback.rs @@ -9,16 +9,31 @@ pub async fn run_rollback_erase(app: AppHandle) -> Result Result { - let config_dir = - store::ensure_config_dir_exists(&app).map_err(|e| capture_err("darwin_build_check", e))?; + crate::attention::begin_build_check(&app); + let config_dir = store::ensure_config_dir_exists(&app).map_err(|error| { + crate::attention::build_check_failed(&app); + capture_err("darwin_build_check", error) + })?; let host_attr = store::get_host_attr(&app) - .map_err(|e| capture_err("darwin_build_check", e))? - .ok_or_else(|| "No host configured — cannot run build check".to_string())?; + .map_err(|error| { + crate::attention::build_check_failed(&app); + capture_err("darwin_build_check", error) + })? + .ok_or_else(|| { + crate::attention::build_check_failed(&app); + "No host configured — cannot run build check".to_string() + })?; let (passed, stdout, stderr) = rebuild::dry_run_build_check(&config_dir, &host_attr, false) - .map_err(|e| capture_err("darwin_build_check", e))?; + .map_err(|error| { + crate::attention::build_check_failed(&app); + capture_err("darwin_build_check", error) + })?; let output = if stderr.is_empty() { stdout } else { stderr }; + if !passed { + crate::attention::build_check_failed(&app); + } Ok(shared_types::BuildCheckResult { passed, output }) } diff --git a/apps/native/src-tauri/src/commands/summarize.rs b/apps/native/src-tauri/src/commands/summarize.rs index ff068debb..b37cba827 100644 --- a/apps/native/src-tauri/src/commands/summarize.rs +++ b/apps/native/src-tauri/src/commands/summarize.rs @@ -74,10 +74,31 @@ pub async fn run_abort_restore(app: AppHandle) -> Result<(), String> { } pub async fn run_finalize_restore(app: AppHandle, target_hash: String) -> Result<(), String> { - rebuild::finalize_restore(&app, target_hash) - .await - .map(|_| ()) - .map_err(|e| capture_err("finalize_restore", e)) + finish_history_restore( + rebuild::finalize_restore(&app, target_hash), + || crate::attention::restore_finished(&app), + || crate::attention::restore_finalization_failed(&app), + ) + .await +} + +// The successful rebuild event stays silent until restore bookkeeping finishes. +// Keep both outcomes here so a finalization failure never also announces success. +async fn finish_history_restore( + finalize: impl std::future::Future>, + on_success: impl FnOnce(), + on_failure: impl FnOnce(), +) -> Result<(), String> { + match finalize.await { + Ok(_) => { + on_success(); + Ok(()) + } + Err(error) => { + on_failure(); + Err(capture_err("finalize_restore", error)) + } + } } pub async fn run_generate_commit_message(app: AppHandle) -> Result { @@ -140,3 +161,51 @@ pub async fn finalize_restore(app: AppHandle, target_hash: String) -> Result<(), pub async fn generate_commit_message(app: AppHandle) -> Result { run_generate_commit_message(app).await } + +#[cfg(test)] +mod tests { + use super::*; + use std::cell::RefCell; + use std::future::Future; + use std::task::Poll; + + #[tokio::test] + async fn history_restore_notifies_once_only_after_finalization_succeeds() { + let notices = RefCell::new(Vec::new()); + let (complete, pending) = tokio::sync::oneshot::channel::>(); + let mut finalize = Box::pin(finish_history_restore( + async { pending.await.unwrap() }, + || notices.borrow_mut().push("finished"), + || notices.borrow_mut().push("finalization_failed"), + )); + + let first_poll = std::future::poll_fn(|cx| Poll::Ready(finalize.as_mut().poll(cx))).await; + assert!(first_poll.is_pending()); + assert!(notices.borrow().is_empty()); + + complete.send(Ok(())).unwrap(); + assert!(finalize.await.is_ok()); + assert_eq!(*notices.borrow(), ["finished"]); + } + + #[tokio::test] + async fn history_restore_finalization_failure_notifies_without_announcing_success() { + let notices = RefCell::new(Vec::new()); + let result = finish_history_restore( + async { + Err::<(), _>(anyhow::anyhow!( + "Failed to record build state after restore" + )) + }, + || notices.borrow_mut().push("finished"), + || notices.borrow_mut().push("finalization_failed"), + ) + .await; + + assert_eq!( + result.unwrap_err(), + "Failed to record build state after restore" + ); + assert_eq!(*notices.borrow(), ["finalization_failed"]); + } +} diff --git a/apps/native/src-tauri/src/evolve/lifecycle.rs b/apps/native/src-tauri/src/evolve/lifecycle.rs index 76b6b6fb8..bb3409aa1 100644 --- a/apps/native/src-tauri/src/evolve/lifecycle.rs +++ b/apps/native/src-tauri/src/evolve/lifecycle.rs @@ -266,6 +266,7 @@ pub async fn backup_evolve_and_record_changeset( evolution.summary.clone(), ), ); + crate::attention::evolution_completed(app, &telemetry.state, true); return Ok(EvolutionResult { change_map: SemanticChangeMap::default(), git_status: initial_status, @@ -332,6 +333,7 @@ pub async fn backup_evolve_and_record_changeset( None, ), ); + crate::attention::evolution_completed(app, &telemetry.state, false); Ok(EvolutionResult { change_map, diff --git a/apps/native/src-tauri/src/evolve/nixmac_ignore.rs b/apps/native/src-tauri/src/evolve/nixmac_ignore.rs index 4363f8b98..d10352427 100644 --- a/apps/native/src-tauri/src/evolve/nixmac_ignore.rs +++ b/apps/native/src-tauri/src/evolve/nixmac_ignore.rs @@ -96,19 +96,9 @@ impl NixmacIgnoreChecker { return false; } - let ignored = matcher + matcher .matched_path_or_any_parents(relative_path, is_dir) - .is_ignore(); - - // This diagnostic is very noisy so it's commented out by default, but it's useful - // for validating the ignore rules. - // log::debug!( - // "nixmac ignore check: {} (dir={}) => {}", - // relative_path.display(), - // is_dir, - // ignored - // ); - ignored + .is_ignore() } } diff --git a/apps/native/src-tauri/src/evolve/session_control.rs b/apps/native/src-tauri/src/evolve/session_control.rs index 75e54e35b..b43b5510c 100644 --- a/apps/native/src-tauri/src/evolve/session_control.rs +++ b/apps/native/src-tauri/src/evolve/session_control.rs @@ -8,6 +8,26 @@ pub const EVOLUTION_CANCELLED_MSG: &str = "Evolution cancelled by user"; /// Global flag to signal evolution cancellation. static EVOLVE_CANCELLED: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false); +static ACTIVE_EVOLUTIONS: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); + +pub struct ActiveEvolutionGuard; + +impl ActiveEvolutionGuard { + pub fn begin() -> Self { + ACTIVE_EVOLUTIONS.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + Self + } +} + +impl Drop for ActiveEvolutionGuard { + fn drop(&mut self) { + ACTIVE_EVOLUTIONS.fetch_sub(1, std::sync::atomic::Ordering::SeqCst); + } +} + +pub fn is_evolve_active() -> bool { + ACTIVE_EVOLUTIONS.load(std::sync::atomic::Ordering::SeqCst) > 0 +} /// Check if evolution has been cancelled. pub fn is_evolve_cancelled() -> bool { diff --git a/apps/native/src-tauri/src/feedback.rs b/apps/native/src-tauri/src/feedback.rs index 1af1db6f4..dc63a2e05 100644 --- a/apps/native/src-tauri/src/feedback.rs +++ b/apps/native/src-tauri/src/feedback.rs @@ -390,6 +390,7 @@ pub fn gather_app_state(app: &AppHandle, feedback_type: &str) -> Value { "configDir": config_dir, "hostAttr": host_attr, "feedbackType": feedback_type, + "helperFailure": crate::system::helper_permission::last_failure_detail(), "evolution": evolution_state, "timestamp": Utc::now().to_rfc3339(), }) @@ -849,6 +850,7 @@ regex = "token=([A-Za-z0-9]+)" let mut metadata = empty_metadata(); metadata.current_app_state_snapshot = Some(json!({ "token": "token=abc123", + "helperFailure": "SMAppServiceErrorDomain 1: token=abc123", "nested": { "value": "token=xyz" } })); metadata.ai_provider_model_info = Some(types::FeedbackAiProviderModelInfo { @@ -868,6 +870,9 @@ regex = "token=([A-Za-z0-9]+)" assert!(redacted_fields.contains(&"ai_provider_model_info")); let state = metadata.current_app_state_snapshot.unwrap(); + let helper_failure = state["helperFailure"].as_str().unwrap(); + assert!(helper_failure.contains("SMAppServiceErrorDomain 1")); + assert!(!helper_failure.contains("abc123")); assert!( state .get("token") diff --git a/apps/native/src-tauri/src/main.rs b/apps/native/src-tauri/src/main.rs index 6e682b57a..580f10779 100644 --- a/apps/native/src-tauri/src/main.rs +++ b/apps/native/src-tauri/src/main.rs @@ -10,6 +10,7 @@ // Keep top-level declarations scoped to first-level domain modules. Leaf modules // are declared by their parent `mod.rs` files so rust-analyzer resolves them via Cargo. mod ai; +mod attention; mod bootstrap; // Consumed by `build.rs` via include!; declared here so its resolution table // stays under `cargo test`. @@ -28,6 +29,7 @@ mod feedback; mod git; mod history; mod http_client; +mod main_window; mod managed_edits; mod observable; mod orpc; @@ -63,7 +65,7 @@ use std::time::Duration; use tauri::{ Emitter, Manager, RunEvent, WebviewUrl, WebviewWindow, WebviewWindowBuilder, WindowEvent, menu::{Menu, MenuItem, PredefinedMenuItem}, - tray::TrayIconBuilder, + tray::{MouseButton, MouseButtonState, TrayIconBuilder, TrayIconEvent}, webview::PageLoadEvent, }; use tracing_subscriber::{EnvFilter, fmt, layer::SubscriberExt, util::SubscriberInitExt}; @@ -500,6 +502,20 @@ fn run_cli_mode(context: tauri::Context) -> i32 { } } +const POPOVER_MAIN_WINDOW_STATE_LABEL: &str = "popover-main"; + +fn window_state_label(label: &str, popover_mode: bool) -> &str { + if label == "main" && popover_mode { + POPOVER_MAIN_WINDOW_STATE_LABEL + } else { + label + } +} + +fn should_track_window_state(label: &str) -> bool { + label != POPOVER_MAIN_WINDOW_STATE_LABEL +} + fn run_gui_mode( context: tauri::Context, log_guard: std::sync::Arc< @@ -513,6 +529,9 @@ fn run_gui_mode( let mut builder = tauri::Builder::default().plugin(tauri_plugin_http::init()); let orpc_router = orpc::build_router(); + let popover_window_state = Arc::new(AtomicBool::new(false)); + let popover_window_state_for_label = Arc::clone(&popover_window_state); + let popover_window_state_for_setup = Arc::clone(&popover_window_state); // The updater will misbehave in dev mode (always says an update is available, fails signature // checks, tries to downgrade your app, etc.), so we only include it in release builds. @@ -542,11 +561,28 @@ fn run_gui_mode( .plugin(tauri_plugin_keyring::init()) .plugin(tauri_plugin_positioner::init()) .plugin(tauri_plugin_process::init()) - .plugin(tauri_plugin_single_instance::init(|_, _, _| {})) + .plugin(tauri_plugin_single_instance::init(|app, _, _| { + let mode = main_window::active(app); + if mode.is_popover() + && let Err(error) = main_window::show(app, mode) + { + log::warn!("Failed to show main window from second app open: {}", error); + } + })) .plugin(tauri_plugin_websocket::init()) .plugin( tauri_plugin_window_state::Builder::new() .with_state_flags(tauri_plugin_window_state::StateFlags::POSITION) + // The plugin still walks its preloaded cache on Exit after a + // window is filtered. Remap popover `main` first so that pass + // cannot overwrite the saved control-window position. + .map_label(move |label| { + window_state_label( + label, + popover_window_state_for_label.load(Ordering::SeqCst), + ) + }) + .with_filter(should_track_window_state) .build(), ) .plugin(tauri_plugin_sql::Builder::new().build()) @@ -695,6 +731,15 @@ fn run_gui_mode( panic_handler::setup_panic_hook(handle.clone()); register_managed_state(handle)?; + let main_window_mode = main_window::read_at_launch(handle); + popover_window_state_for_setup.store(main_window_mode.is_popover(), Ordering::SeqCst); + app.manage(main_window_mode); + log::info!("Main window mode at launch: {:?}", main_window_mode); + if let Err(error) = attention::install(handle) { + // Attention is additive UX. A notification-center integration + // failure must not prevent the user from opening nixmac. + log::warn!("Could not install menu-bar attention notifications: {error}"); + } // Initialize SQLite database before any consumer that reads the // managed DbPool from app state. @@ -758,56 +803,113 @@ fn run_gui_mode( app.manage(telemetry::init::init_telemetry(send_diagnostics)); // Build the system tray menu - let open_i = MenuItem::with_id(app, "open", "Open nixmac", true, None::<&str>)?; - let sep1 = PredefinedMenuItem::separator(app)?; let feedback_i = MenuItem::with_id(app, "send_feedback", "Send Feedback...", true, None::<&str>)?; let settings_i = MenuItem::with_id(app, "settings", "Settings...", true, None::<&str>)?; let sep2 = PredefinedMenuItem::separator(app)?; let quit_i = MenuItem::with_id(app, "quit", "Quit nixmac", true, None::<&str>)?; - let menu = Menu::with_items( - app, - &[&open_i, &sep1, &feedback_i, &settings_i, &sep2, &quit_i], - )?; + let menu = if main_window_mode.is_popover() { + Menu::with_items(app, &[&feedback_i, &settings_i, &sep2, &quit_i])? + } else { + let open_i = MenuItem::with_id(app, "open", "Open nixmac", true, None::<&str>)?; + let sep1 = PredefinedMenuItem::separator(app)?; + Menu::with_items( + app, + &[&open_i, &sep1, &feedback_i, &settings_i, &sep2, &quit_i], + )? + }; // Clone a handle to the guard for use in menu callbacks (so we can flush on quit) let log_guard_for_menu = log_guard.clone(); + let mode_for_tray = main_window_mode; + let mode_for_menu = main_window_mode; + let tray_click_state = Arc::new(Mutex::new(main_window::TrayClickState::default())); + let tray_click_state_for_tray = Arc::clone(&tray_click_state); - let _tray = TrayIconBuilder::new() + let _tray = TrayIconBuilder::with_id(main_window::MAIN_TRAY_ID) .icon(tray_icon::load()?) .icon_as_template(true) .menu(&menu) - .show_menu_on_left_click(true) - .on_tray_icon_event(|tray_handle, event| { + .show_menu_on_left_click(!main_window_mode.is_popover()) + .on_tray_icon_event(move |tray_handle, event| { tauri_plugin_positioner::on_tray_event(tray_handle.app_handle(), &event); + if !mode_for_tray.is_popover() { + return; + } + let input = match event { + TrayIconEvent::Click { + button: MouseButton::Left, + button_state: MouseButtonState::Down, + .. + } => { + let (is_visible, is_minimized) = tray_handle + .app_handle() + .get_webview_window("main") + .map(|window| { + ( + window.is_visible().unwrap_or(false), + window.is_minimized().unwrap_or(true), + ) + }) + .unwrap_or((false, false)); + main_window::TrayClickInput::Down { + is_visible, + is_minimized, + } + } + TrayIconEvent::Click { + button: MouseButton::Left, + button_state: MouseButtonState::Up, + .. + } => main_window::TrayClickInput::Up, + _ => return, + }; + let decision = { + let mut state = tray_click_state_for_tray + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + let decision = main_window::decide_tray_click( + mode_for_tray, + *state, + input, + ); + *state = decision.state; + decision + }; + log::debug!("Menu-bar popover tray transition: {:?}", decision); + let result = match decision.action { + main_window::TrayClickAction::None => return, + main_window::TrayClickAction::Show => { + main_window::show(tray_handle.app_handle(), mode_for_tray) + } + main_window::TrayClickAction::Dismiss => { + main_window::dismiss(tray_handle.app_handle(), mode_for_tray).map(|_| ()) + } + }; + if let Err(error) = result { + log::warn!("Failed to toggle menu-bar popover: {}", error); + } }) - // All window calls below are fire-and-forget: tray menu callbacks run - // asynchronously and the window may be hidden or in a transitional state. - // show/set_focus/emit only fail when the window is destroyed, which is - // acceptable — the app is still running, just with no visible window. + // Menu callbacks can race app shutdown, so their window actions are best-effort. .on_menu_event(move |app, event| match event.id().as_ref() { "open" => { - if let Some(window) = app.get_webview_window("main") { - let _ = window.show(); - let _ = window.set_focus(); - } + let _ = main_window::show(app, mode_for_menu); } "send_feedback" => { + let _ = main_window::show(app, mode_for_menu); if let Some(window) = app.get_webview_window("main") { - let _ = window.show(); - let _ = window.set_focus(); let _ = window.emit("tray:open-feedback", ()); } } "settings" => { + let _ = main_window::show(app, mode_for_menu); if let Some(window) = app.get_webview_window("main") { - let _ = window.show(); - let _ = window.set_focus(); let _ = window.emit("tray:open-settings", ()); } } "quit" => { + attention::shutdown(); // Explicitly drop the WorkerGuard (flush logs) before exiting. if let Some(_g) = log_guard_for_menu.lock().unwrap().take() { // `_g` dropped here @@ -820,11 +922,23 @@ fn run_gui_mode( // Create the main window let initial_width = 800.0; - let initial_height = 800.0; + let initial_height = if main_window_mode.is_popover() { + 600.0 + } else { + 800.0 + }; let min_width = 800.0; - let max_width = 2400.0; + let max_width = if main_window_mode.is_popover() { + 800.0 + } else { + 2400.0 + }; let min_height = 600.0; - let max_height = 1800.0; + let max_height = if main_window_mode.is_popover() { + 600.0 + } else { + 1800.0 + }; let e2e_opaque_window = e2e_opaque_window_enabled(); let e2e_solid_capture = e2e_solid_capture_enabled(); let e2e_css_capture = e2e_solid_capture || e2e_opaque_window; @@ -840,7 +954,10 @@ fn run_gui_mode( } let main_webview_loaded = Arc::new(AtomicBool::new(false)); let main_webview_loaded_for_page_load = Arc::clone(&main_webview_loaded); + let initial_popover_shown = Arc::new(AtomicBool::new(false)); + let initial_popover_shown_for_page_load = Arc::clone(&initial_popover_shown); let e2e_page_load_boot_probe = e2e_webview_watchdog; + let mode_for_page_load = main_window_mode; let mut main_window_builder = WebviewWindowBuilder::new(app, "main", WebviewUrl::App("index.html".into())) @@ -848,14 +965,14 @@ fn run_gui_mode( .inner_size(initial_width, initial_height) .min_inner_size(min_width, min_height) .max_inner_size(max_width, max_height) - .resizable(true) + .resizable(!main_window_mode.is_popover()) .maximizable(false) .minimizable(true) .closable(true) .decorations(true) .transparent(!e2e_opaque_window) .theme(Some(tauri::Theme::Dark)) - .visible(true) + .visible(!main_window_mode.is_popover()) .background_color(tauri::utils::config::Color(0, 0, 0, 235)) .always_on_top(false) .visible_on_all_workspaces(true) @@ -867,6 +984,15 @@ fn run_gui_mode( ); if payload.event() == PageLoadEvent::Finished { main_webview_loaded_for_page_load.store(true, Ordering::SeqCst); + if mode_for_page_load.is_popover() + && !initial_popover_shown_for_page_load + .swap(true, Ordering::SeqCst) + { + main_window::show_initial_when_ready( + window.app_handle(), + mode_for_page_load, + ); + } if e2e_page_load_boot_probe { e2e_schedule_webview_boot_probe( window.clone(), @@ -912,6 +1038,10 @@ fn run_gui_mode( })?; log::debug!("Main nixmac window created"); + if let Err(error) = main_window::install_macos_event_handlers(handle) { + log::warn!("Could not install menu-bar popover event handlers: {}", error); + } + // Frosted-glass window background via native AppKit vibrancy // (NSVisualEffectView). The main webview is transparent and the CSS // only paints a translucent tint; without a native backing layer, @@ -1029,14 +1159,17 @@ use window_vibrancy::{apply_vibrancy, NSVisualEffectMaterial, NSVisualEffectStat let _ = main_window; // Create the preview indicator window (persistent banner for uncommitted changes) - if let Err(e) = peek::create_preview_indicator_window(handle) { + if main_window_mode.shows_detached_indicators() + && let Err(e) = peek::create_preview_indicator_window(handle) + { log::error!("[peek] ❌ Failed to create preview indicator window: {}", e); } // Experimental: create the spinning-mascot indicator window when the // flag is enabled at launch. Gated here so users who never enable it // pay no startup cost; enabling the flag applies on the next launch. - if crate::state::ui_prefs::experimental_spinning_mascot(handle) + if main_window_mode.shows_detached_indicators() + && crate::state::ui_prefs::experimental_spinning_mascot(handle) && let Err(e) = peek::create_evolve_mascot_window(handle) { log::error!("[peek] failed to create evolve mascot window: {}", e); } @@ -1052,6 +1185,10 @@ use window_vibrancy::{apply_vibrancy, NSVisualEffectMaterial, NSVisualEffectStat .build(context) .expect("error while building tauri application") .run(|app_handle, event| { + if matches!(event, RunEvent::Exit) { + attention::shutdown(); + } + // Handle window close events - hide window but keep app running if let RunEvent::WindowEvent { label, @@ -1061,10 +1198,13 @@ use window_vibrancy::{apply_vibrancy, NSVisualEffectMaterial, NSVisualEffectStat && label == "main" { // Prevent the window from being destroyed api.prevent_close(); - if let Some(window) = app_handle.get_webview_window("main") { - // fire-and-forget: hide() can fail if window is already hidden. + let mode = main_window::active(app_handle); + if mode.is_popover() { + if let Err(error) = main_window::request_close(app_handle) { + log::warn!("Failed to hand popover close request to the WebView: {}", error); + } + } else if let Some(window) = app_handle.get_webview_window("main") { let _ = window.hide(); - // Update peek state peek::unlock_and_hide(); } } @@ -1072,12 +1212,10 @@ use window_vibrancy::{apply_vibrancy, NSVisualEffectMaterial, NSVisualEffectStat #[cfg(target_os = "macos")] { // Click Nixmac icon to show - if let RunEvent::Reopen { .. } = &event - && let Some(window) = app_handle.get_webview_window("main") { - // fire-and-forget: show/set_focus fail only on destroyed window. - let _ = window.show(); - let _ = window.set_focus(); - } + if let RunEvent::Reopen { .. } = &event { + // fire-and-forget: show/focus only fail on a destroyed window. + let _ = main_window::show(app_handle, main_window::active(app_handle)); + } } }); } @@ -1089,6 +1227,21 @@ mod managed_state_tests { use crate::shared_types::GlobalPreferences; use tauri::Manager; + #[test] + fn popover_position_does_not_replace_the_saved_control_position() { + assert_eq!(window_state_label("main", false), "main"); + assert_eq!( + window_state_label("main", true), + POPOVER_MAIN_WINDOW_STATE_LABEL + ); + assert!(!should_track_window_state(window_state_label("main", true))); + assert!(should_track_window_state(window_state_label("main", false))); + assert!(should_track_window_state(window_state_label( + "preview-indicator", + true + ))); + } + /// Regression for the PR #411 CLI breakage: `run_cli_mode` calls /// `Builder::build()` but never starts the event loop, and Tauri only runs /// `.setup()` from `RunEvent::Ready`. So the `GlobalPreferences` observable diff --git a/apps/native/src-tauri/src/main_window.rs b/apps/native/src-tauri/src/main_window.rs new file mode 100644 index 000000000..bf3264ad2 --- /dev/null +++ b/apps/native/src-tauri/src/main_window.rs @@ -0,0 +1,1588 @@ +//! Launch-scoped control and menu-bar popover behavior for the main window. + +use serde::Serialize; +use std::sync::atomic::{AtomicBool, AtomicU32, AtomicU64, Ordering}; +use std::time::Duration; +use tauri::{AppHandle, Emitter, Manager, Runtime, WebviewWindow}; + +#[cfg(target_os = "macos")] +use block2::RcBlock; +#[cfg(target_os = "macos")] +use objc2::{ + rc::Retained, + runtime::{AnyObject, ProtocolObject}, +}; +#[cfg(target_os = "macos")] +use objc2_app_kit::{ + NSApplication, NSApplicationDidBecomeActiveNotification, + NSApplicationDidChangeScreenParametersNotification, NSApplicationDidResignActiveNotification, + NSEvent, NSEventMask, NSResponder, NSWindow, +}; +#[cfg(target_os = "macos")] +use objc2_foundation::{ + MainThreadMarker, NSNotification, NSNotificationCenter, NSObjectProtocol, NSPoint, NSRect, +}; +#[cfg(target_os = "macos")] +use std::{ + cell::RefCell, + ptr::NonNull, + sync::{ + Arc, + atomic::{AtomicU8, Ordering as AtomicOrdering}, + mpsc, + }, +}; + +pub(crate) const MAIN_TRAY_ID: &str = "main-tray"; +pub(crate) const MENU_BAR_POPOVER_FLAG: &str = "menu-bar-popover"; + +const E2E_CONTROL_WINDOW_KEYS: &[&str] = &[ + "NIXMAC_E2E_MOCK_SYSTEM", + "NIXMAC_E2E_SOLID_CAPTURE", + "NIXMAC_E2E_OPAQUE_WINDOW", + "NIXMAC_E2E_WEBVIEW_WATCHDOG", + "NIXMAC_E2E_CONFIG_DIR", + "NIXMAC_E2E_DIAGNOSTICS_DIR", +]; +// NSStatusItem geometry can lag page load on a cold launch. Keep the window +// hidden for up to five seconds instead of flashing at a stale/default frame. +const INITIAL_POPOVER_POSITION_ATTEMPTS: usize = 100; +const INITIAL_POPOVER_POSITION_RETRY: Duration = Duration::from_millis(50); +const CLOSE_REQUEST_FALLBACK: Duration = Duration::from_secs(1); +#[cfg(target_os = "macos")] +const MACOS_ESCAPE_KEY_CODE: u16 = 53; +#[cfg(target_os = "macos")] +const MAX_RESPONDER_CHAIN_DEPTH: usize = 64; +// WKWebView can acquire its responder after the window is ordered onscreen. +const WEBVIEW_REFOCUS_DELAY: Duration = Duration::from_millis(75); + +static REFOCUS_GENERATION: AtomicU64 = AtomicU64::new(0); +static ACTIVATION_REOPEN_ARMED: AtomicBool = AtomicBool::new(false); +static CLOSE_REQUESTS: CloseRequestTracker = CloseRequestTracker::new(); + +#[derive(Debug)] +struct CloseRequestTracker { + next_token: AtomicU32, + pending_token: AtomicU32, +} + +impl CloseRequestTracker { + const fn new() -> Self { + Self { + next_token: AtomicU32::new(1), + pending_token: AtomicU32::new(0), + } + } + + fn begin(&self) -> u32 { + // Zero denotes no pending close. A wrapped counter simply skips it; + // only the latest token can authorize its one-second fallback. + let token = loop { + let token = self.next_token.fetch_add(1, Ordering::SeqCst); + if token != 0 { + break token; + } + }; + self.pending_token.store(token, Ordering::SeqCst); + token + } + + fn retire(&self, token: u32) -> bool { + token != 0 + && self + .pending_token + .compare_exchange(token, 0, Ordering::SeqCst, Ordering::SeqCst) + .is_ok() + } + + fn pending(&self) -> u32 { + self.pending_token.load(Ordering::SeqCst) + } + + /// The caller serializes the current-token check and native hide with + /// show/request_close on the main thread. Retire only the matching token, + /// after hiding succeeds; an error or missing window leaves the fallback. + fn dismiss_if_current( + &self, + token: u32, + hide: impl FnOnce() -> Result, + ) -> Result { + if token == 0 || self.pending() != token { + return Ok(false); + } + let hidden = hide()?; + if hidden { + self.retire(token); + } + Ok(hidden) + } + + fn clear(&self) { + self.pending_token.store(0, Ordering::SeqCst); + } +} + +#[derive(Debug, Clone, Copy, Serialize)] +pub(crate) struct CloseRequestedPayload { + pub(crate) token: u32, +} + +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub(crate) enum MainWindowMode { + #[default] + Control, + Popover, +} + +impl MainWindowMode { + pub(crate) fn is_popover(self) -> bool { + self == Self::Popover + } + + pub(crate) fn shows_detached_indicators(self) -> bool { + !self.is_popover() + } +} + +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub(crate) struct TrayClickState { + started_visible: bool, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum TrayClickInput { + Down { + is_visible: bool, + is_minimized: bool, + }, + Up, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum TrayClickAction { + None, + Show, + Dismiss, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) struct TrayClickDecision { + pub(crate) state: TrayClickState, + pub(crate) action: TrayClickAction, +} + +/// Decide a complete left-click transition without touching AppKit or Tauri. +/// +/// Visibility is sampled on mouse-down so the corresponding mouse-up toggles +/// from the state the user actually clicked. Minimized popovers are treated as +/// hidden, and every mouse-up resets the gesture state. +pub(crate) fn decide_tray_click( + mode: MainWindowMode, + state: TrayClickState, + input: TrayClickInput, +) -> TrayClickDecision { + if !mode.is_popover() { + return TrayClickDecision { + state: TrayClickState::default(), + action: TrayClickAction::None, + }; + } + + match input { + TrayClickInput::Down { + is_visible, + is_minimized, + } => TrayClickDecision { + state: TrayClickState { + started_visible: is_visible && !is_minimized, + }, + action: TrayClickAction::None, + }, + TrayClickInput::Up => TrayClickDecision { + state: TrayClickState::default(), + action: if state.started_visible { + TrayClickAction::Dismiss + } else { + TrayClickAction::Show + }, + }, + } +} + +fn resolve_mode(override_value: Option<&str>, force_e2e_control: bool) -> MainWindowMode { + if force_e2e_control { + return MainWindowMode::Control; + } + match override_value { + Some("popover") => MainWindowMode::Popover, + _ => MainWindowMode::Control, + } +} + +fn e2e_forces_control_window() -> bool { + cfg!(debug_assertions) + && should_force_e2e_control( + E2E_CONTROL_WINDOW_KEYS + .iter() + .any(|key| crate::e2e_runtime::value(key).is_some()), + crate::e2e_runtime::value("NIXMAC_E2E_WINDOW_MODE").as_deref(), + ) +} + +fn should_force_e2e_control(has_e2e_control_key: bool, explicit_mode: Option<&str>) -> bool { + has_e2e_control_key && explicit_mode != Some("popover") +} + +fn resolve_e2e_window_mode(value: Option<&str>, debug_build: bool) -> Option { + if !debug_build { + return None; + } + match value { + Some("popover") => Some(MainWindowMode::Popover), + Some("control") => Some(MainWindowMode::Control), + _ => None, + } +} + +fn e2e_window_mode_override() -> Option { + let value = crate::e2e_runtime::value("NIXMAC_E2E_WINDOW_MODE"); + resolve_e2e_window_mode(value.as_deref(), cfg!(debug_assertions)) +} + +/// Resolve the launch-scoped window mode after `GlobalPreferences` is managed. +/// +/// Rust intentionally reads only the local string override. PostHog flags are +/// JS-only in v1, and every unset or unrecognized value remains control. +pub(crate) fn read_at_launch(app: &AppHandle) -> MainWindowMode { + if let Some(mode) = e2e_window_mode_override() { + return mode; + } + let override_value = crate::state::preferences::try_read(app).and_then(|preferences| { + preferences + .feature_flag_overrides + .and_then(|overrides| overrides.get(MENU_BAR_POPOVER_FLAG).cloned()) + }); + resolve_mode(override_value.as_deref(), e2e_forces_control_window()) +} + +pub(crate) fn active(app: &AppHandle) -> MainWindowMode { + app.try_state::() + .map(|mode| *mode) + .unwrap_or_default() +} + +#[derive(Debug, Clone, Copy, PartialEq)] +#[cfg(any(target_os = "macos", test))] +struct LogicalRect { + x: f64, + y: f64, + width: f64, + height: f64, +} + +#[cfg(target_os = "macos")] +impl LogicalRect { + fn from_ns_rect(rect: NSRect) -> Self { + Self { + x: rect.origin.x, + y: rect.origin.y, + width: rect.size.width, + height: rect.size.height, + } + } +} + +#[cfg(any(target_os = "macos", test))] +fn popover_origin( + status_item: LogicalRect, + window_width: f64, + window_height: f64, + visible_frame: LogicalRect, +) -> (f64, f64) { + let centered_x = status_item.x + status_item.width / 2.0 - window_width / 2.0; + let below_status_item = status_item.y - window_height; + let max_x = (visible_frame.x + visible_frame.width - window_width).max(visible_frame.x); + let max_y = (visible_frame.y + visible_frame.height - window_height).max(visible_frame.y); + + ( + centered_x.clamp(visible_frame.x, max_x), + below_status_item.clamp(visible_frame.y, max_y), + ) +} + +#[cfg(any(target_os = "macos", test))] +fn popover_visible_frame_fallback_origin( + window_width: f64, + window_height: f64, + visible_frame: LogicalRect, +) -> (f64, f64) { + ( + (visible_frame.x + visible_frame.width - window_width).max(visible_frame.x), + (visible_frame.y + visible_frame.height - window_height).max(visible_frame.y), + ) +} + +#[cfg(any(target_os = "macos", test))] +fn status_item_frame_is_usable(status_item: LogicalRect, screen_frame: LogicalRect) -> bool { + const EDGE_TOLERANCE: f64 = 1.0; + const MAX_STATUS_ITEM_WIDTH: f64 = 256.0; + const MAX_STATUS_ITEM_HEIGHT: f64 = 64.0; + let screen_max_x = screen_frame.x + screen_frame.width; + let screen_max_y = screen_frame.y + screen_frame.height; + let status_item_top = status_item.y + status_item.height; + status_item.width > 0.0 + && status_item.width <= MAX_STATUS_ITEM_WIDTH + && status_item.height > 0.0 + && status_item.height <= MAX_STATUS_ITEM_HEIGHT + && status_item.x >= screen_frame.x - EDGE_TOLERANCE + && status_item.x + status_item.width <= screen_max_x + EDGE_TOLERANCE + && status_item.y <= screen_max_y + EDGE_TOLERANCE + && status_item_top >= screen_max_y - EDGE_TOLERANCE + && status_item_top <= screen_max_y + EDGE_TOLERANCE +} + +#[cfg(target_os = "macos")] +fn run_on_main_thread_sync(app: &AppHandle, task: F) -> Result +where + R: Runtime, + T: Send + 'static, + F: FnOnce() -> T + Send + 'static, +{ + if MainThreadMarker::new().is_some() { + return Ok(task()); + } + let (sender, receiver) = mpsc::sync_channel(1); + app.run_on_main_thread(move || { + let _ = sender.send(task()); + }) + .map_err(|error| error.to_string())?; + receiver.recv().map_err(|error| error.to_string()) +} + +#[cfg(target_os = "macos")] +fn position_under_status_item_on_main( + app: &AppHandle, + window: &WebviewWindow, +) -> Result<(), String> { + let tray = app + .tray_by_id(MAIN_TRAY_ID) + .ok_or_else(|| "Main tray icon not found".to_string())?; + let (status_item, visible_frame) = tray + .with_inner_tray_icon(|inner| -> Result<(LogicalRect, LogicalRect), String> { + let mtm = MainThreadMarker::new() + .ok_or_else(|| "Status-item geometry must run on the main thread".to_string())?; + let status_item = inner + .ns_status_item() + .ok_or_else(|| "Native status item is unavailable".to_string())?; + let button = status_item + .button(mtm) + .ok_or_else(|| "Native status-item button is unavailable".to_string())?; + let status_window = button + .window() + .ok_or_else(|| "Native status-item window is unavailable".to_string())?; + let screen = status_window + .screen() + .ok_or_else(|| "Status-item screen is unavailable".to_string())?; + + let status_item = LogicalRect::from_ns_rect(status_window.frame()); + let screen_frame = LogicalRect::from_ns_rect(screen.frame()); + let visible_frame = LogicalRect::from_ns_rect(screen.visibleFrame()); + if !status_item_frame_is_usable(status_item, screen_frame) { + return Err(format!( + "Status-item frame is not ready: ({:.1},{:.1},{:.1}x{:.1})", + status_item.x, status_item.y, status_item.width, status_item.height, + )); + } + + Ok((status_item, visible_frame)) + }) + .map_err(|error| error.to_string())??; + + unsafe { + let ns_window = window.ns_window().map_err(|error| error.to_string())?; + let ns_window = &*(ns_window.cast::()); + let frame = ns_window.frame(); + let (x, y) = popover_origin( + status_item, + frame.size.width, + frame.size.height, + visible_frame, + ); + log::debug!( + "Popover geometry: status=({:.1},{:.1},{:.1}x{:.1}) visible=({:.1},{:.1},{:.1}x{:.1}) window={:.1}x{:.1} origin=({:.1},{:.1})", + status_item.x, + status_item.y, + status_item.width, + status_item.height, + visible_frame.x, + visible_frame.y, + visible_frame.width, + visible_frame.height, + frame.size.width, + frame.size.height, + x, + y, + ); + ns_window.setFrameOrigin(NSPoint::new(x, y)); + } + Ok(()) +} + +#[cfg(target_os = "macos")] +fn position_in_status_screen_visible_frame_on_main( + app: &AppHandle, + window: &WebviewWindow, +) -> Result<(), String> { + let tray = app + .tray_by_id(MAIN_TRAY_ID) + .ok_or_else(|| "Main tray icon not found".to_string())?; + let visible_frame = tray + .with_inner_tray_icon(|inner| -> Result { + let mtm = MainThreadMarker::new() + .ok_or_else(|| "Status-screen fallback must run on the main thread".to_string())?; + let status_item = inner + .ns_status_item() + .ok_or_else(|| "Native status item is unavailable".to_string())?; + let button = status_item + .button(mtm) + .ok_or_else(|| "Native status-item button is unavailable".to_string())?; + let status_window = button + .window() + .ok_or_else(|| "Native status-item window is unavailable".to_string())?; + let screen = status_window + .screen() + .ok_or_else(|| "Status-item screen is unavailable".to_string())?; + Ok(LogicalRect::from_ns_rect(screen.visibleFrame())) + }) + .map_err(|error| error.to_string())??; + + unsafe { + let ns_window = window.ns_window().map_err(|error| error.to_string())?; + let ns_window = &*(ns_window.cast::()); + let frame = ns_window.frame(); + let (x, y) = popover_visible_frame_fallback_origin( + frame.size.width, + frame.size.height, + visible_frame, + ); + ns_window.setFrameOrigin(NSPoint::new(x, y)); + } + Ok(()) +} + +#[cfg(target_os = "macos")] +fn position_in_status_screen_visible_frame( + app: &AppHandle, + window: &WebviewWindow, +) -> Result<(), String> { + if MainThreadMarker::new().is_some() { + return position_in_status_screen_visible_frame_on_main(app, window); + } + let task_app = app.clone(); + let task_window = window.clone(); + run_on_main_thread_sync(app, move || { + position_in_status_screen_visible_frame_on_main(&task_app, &task_window) + })? +} + +#[cfg(not(target_os = "macos"))] +fn position_in_status_screen_visible_frame( + _app: &AppHandle, + _window: &WebviewWindow, +) -> Result<(), String> { + Ok(()) +} + +#[cfg(target_os = "macos")] +fn position_under_status_item( + app: &AppHandle, + window: &WebviewWindow, +) -> Result<(), String> { + if MainThreadMarker::new().is_some() { + return position_under_status_item_on_main(app, window); + } + let task_app = app.clone(); + let task_window = window.clone(); + run_on_main_thread_sync(app, move || { + position_under_status_item_on_main(&task_app, &task_window) + })? +} + +#[cfg(not(target_os = "macos"))] +fn position_under_status_item( + _app: &AppHandle, + _window: &WebviewWindow, +) -> Result<(), String> { + Ok(()) +} + +#[cfg(target_os = "macos")] +fn native_window_has_owned_dialog(ns_window: &NSWindow, mtm: MainThreadMarker) -> bool { + ns_window.attachedSheet().is_some() + || NSApplication::sharedApplication(mtm) + .modalWindow() + .is_some() +} + +#[cfg(target_os = "macos")] +fn responder_chain_contains( + mut responder: Option>, + expected_responder: &NSResponder, +) -> bool { + for _ in 0..MAX_RESPONDER_CHAIN_DEPTH { + let Some(current) = responder else { + return false; + }; + if std::ptr::eq(&*current, expected_responder) { + return true; + } + responder = unsafe { current.nextResponder() }; + } + false +} + +#[cfg(target_os = "macos")] +fn make_webview_first_responder(window: &WebviewWindow) -> Result<(), String> { + window + .with_webview(|webview| { + let Some(mtm) = MainThreadMarker::new() else { + log::debug!("Skipping WebView focus outside the main thread"); + return; + }; + unsafe { + let ns_window = &*(webview.ns_window().cast::()); + let webview_responder = &*(webview.inner().cast::()); + if ns_window.isVisible() + && ns_window.isKeyWindow() + && !native_window_has_owned_dialog(ns_window, mtm) + { + ns_window.makeFirstResponder(Some(webview_responder)); + } + } + }) + .map_err(|error| error.to_string()) +} + +#[cfg(not(target_os = "macos"))] +fn make_webview_first_responder(_window: &WebviewWindow) -> Result<(), String> { + Ok(()) +} + +fn should_run_delayed_refocus( + captured_generation: u64, + current_generation: u64, + mode: MainWindowMode, + is_visible: bool, +) -> bool { + captured_generation == current_generation && mode.is_popover() && is_visible +} + +fn should_continue_initial_show( + startup_generation: u64, + current_generation: u64, + mode: MainWindowMode, +) -> bool { + startup_generation == current_generation && mode.is_popover() +} + +fn claim_initial_show_generation(generation: &AtomicU64) -> Option { + generation + .compare_exchange(0, 1, Ordering::SeqCst, Ordering::SeqCst) + .ok() + .map(|_| 1) +} + +#[cfg(any(target_os = "macos", test))] +fn should_reanchor_after_screen_change(mode: MainWindowMode, is_visible: bool) -> bool { + mode.is_popover() && is_visible +} + +#[cfg(any(target_os = "macos", test))] +fn should_reopen_after_activation( + mode: MainWindowMode, + is_visible: bool, + activation_reopen_armed: bool, + pointer_buttons_pressed: bool, +) -> bool { + mode.is_popover() && !is_visible && activation_reopen_armed && !pointer_buttons_pressed +} + +fn schedule_delayed_refocus( + app: &AppHandle, + window: &WebviewWindow, + generation: u64, +) { + let app = app.clone(); + let window = window.clone(); + std::thread::spawn(move || { + std::thread::sleep(WEBVIEW_REFOCUS_DELAY); + let task_app = app.clone(); + let task_window = window.clone(); + if let Err(error) = app.run_on_main_thread(move || { + let current_generation = REFOCUS_GENERATION.load(Ordering::SeqCst); + let is_visible = task_window.is_visible().unwrap_or(false); + if should_run_delayed_refocus( + generation, + current_generation, + active(&task_app), + is_visible, + ) && let Err(error) = make_webview_first_responder(&task_window) + { + log::debug!("Delayed WebView refocus failed: {}", error); + } + }) { + log::debug!("Could not schedule delayed WebView refocus: {}", error); + } + }); +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[cfg(any(target_os = "macos", test))] +enum EscapeAction { + PassThrough, + BridgeToWebview, +} + +#[derive(Debug, Clone, Copy)] +#[cfg(any(target_os = "macos", test))] +struct EscapeContext { + is_popover: bool, + is_visible: bool, + is_key_window: bool, + event_targets_main_window: bool, + has_owned_native_dialog: bool, + webview_owns_first_responder: bool, +} + +#[cfg(any(target_os = "macos", test))] +fn escape_action(context: EscapeContext) -> EscapeAction { + if context.is_popover + && context.is_visible + && context.is_key_window + && context.event_targets_main_window + && !context.has_owned_native_dialog + && !context.webview_owns_first_responder + { + EscapeAction::BridgeToWebview + } else { + EscapeAction::PassThrough + } +} + +#[cfg(target_os = "macos")] +struct MacWindowObservers { + escape_monitor: Retained, + become_active_observer: Retained>, + resign_observer: Retained>, + screen_parameters_observer: Retained>, +} + +#[cfg(target_os = "macos")] +impl Drop for MacWindowObservers { + fn drop(&mut self) { + unsafe { + NSEvent::removeMonitor(&self.escape_monitor); + let observer: &AnyObject = (*self.become_active_observer).as_ref(); + NSNotificationCenter::defaultCenter().removeObserver(observer); + let observer: &AnyObject = (*self.resign_observer).as_ref(); + NSNotificationCenter::defaultCenter().removeObserver(observer); + let observer: &AnyObject = (*self.screen_parameters_observer).as_ref(); + NSNotificationCenter::defaultCenter().removeObserver(observer); + } + } +} + +#[cfg(target_os = "macos")] +thread_local! { + static MAC_WINDOW_OBSERVERS: RefCell> = const { RefCell::new(None) }; +} + +/// Install the macOS event bridge and application-activation observer. +/// +/// The local monitor passes WebView-owned Escape events through untouched. It +/// only consumes an early Escape that targets the main window before WKWebView +/// owns first responder, after successfully bridging that event to the frontend. +#[cfg(target_os = "macos")] +pub(crate) fn install_macos_event_handlers(app: &AppHandle) -> Result<(), String> { + if !active(app).is_popover() { + return Ok(()); + } + + let escape_app = app.clone(); + let escape_handler = RcBlock::new(move |event: NonNull| -> *mut NSEvent { + let event_ref = unsafe { event.as_ref() }; + if event_ref.keyCode() != MACOS_ESCAPE_KEY_CODE || !active(&escape_app).is_popover() { + return event.as_ptr(); + } + + let Some(window) = escape_app.get_webview_window("main") else { + return event.as_ptr(); + }; + let action = Arc::new(AtomicU8::new(0)); + let action_for_webview = Arc::clone(&action); + let event_window_number = event_ref.windowNumber(); + if window + .with_webview(move |webview| unsafe { + let Some(mtm) = MainThreadMarker::new() else { + return; + }; + let ns_window = &*(webview.ns_window().cast::()); + let webview_responder = &*(webview.inner().cast::()); + let context = EscapeContext { + is_popover: true, + is_visible: ns_window.isVisible(), + is_key_window: ns_window.isKeyWindow(), + event_targets_main_window: event_window_number == ns_window.windowNumber(), + has_owned_native_dialog: native_window_has_owned_dialog(ns_window, mtm), + webview_owns_first_responder: responder_chain_contains( + ns_window.firstResponder(), + webview_responder, + ), + }; + let value = match escape_action(context) { + EscapeAction::PassThrough => 1, + EscapeAction::BridgeToWebview => 2, + }; + action_for_webview.store(value, AtomicOrdering::SeqCst); + }) + .is_err() + { + return event.as_ptr(); + } + + if action.load(AtomicOrdering::SeqCst) == 2 { + if let Err(error) = window.emit("window:escape", ()) { + log::warn!("Failed to bridge early Escape to the WebView: {}", error); + return event.as_ptr(); + } + return std::ptr::null_mut(); + } + event.as_ptr() + }); + let escape_monitor = unsafe { + NSEvent::addLocalMonitorForEventsMatchingMask_handler(NSEventMask::KeyDown, &escape_handler) + } + .ok_or_else(|| "Failed to install menu-bar popover Escape monitor".to_string())?; + + let resign_app = app.clone(); + let resign_handler = RcBlock::new(move |_notification: NonNull| { + if active(&resign_app).is_popover() + && let Err(error) = dismiss(&resign_app, MainWindowMode::Popover) + { + log::warn!( + "Failed to dismiss menu-bar popover after app resign: {}", + error + ); + } + }); + let center = NSNotificationCenter::defaultCenter(); + let become_active_app = app.clone(); + let become_active_handler = RcBlock::new(move |_notification: NonNull| { + let Some(window) = become_active_app.get_webview_window("main") else { + return; + }; + let is_visible = window.is_visible().unwrap_or(false); + // Pointer-driven activation already has an explicit owner: the status-item + // click toggler, Dock reopen event, or tray menu callback. Skipping it here + // prevents a mouse-down activation from showing the window before the + // corresponding mouse-up decides whether to show or dismiss it. + let pointer_buttons_pressed = NSEvent::pressedMouseButtons() != 0; + if should_reopen_after_activation( + active(&become_active_app), + is_visible, + ACTIVATION_REOPEN_ARMED.load(Ordering::SeqCst), + pointer_buttons_pressed, + ) && let Err(error) = show(&become_active_app, MainWindowMode::Popover) + { + log::warn!( + "Failed to reopen menu-bar popover after app activation: {}", + error + ); + } + }); + let become_active_observer = unsafe { + center.addObserverForName_object_queue_usingBlock( + Some(NSApplicationDidBecomeActiveNotification), + None, + None, + &become_active_handler, + ) + }; + + let resign_observer = unsafe { + center.addObserverForName_object_queue_usingBlock( + Some(NSApplicationDidResignActiveNotification), + None, + None, + &resign_handler, + ) + }; + + let screen_parameters_app = app.clone(); + let screen_parameters_handler = RcBlock::new(move |_notification: NonNull| { + let Some(window) = screen_parameters_app.get_webview_window("main") else { + return; + }; + let is_visible = window.is_visible().unwrap_or(false); + if should_reanchor_after_screen_change(active(&screen_parameters_app), is_visible) + && let Err(error) = position_under_status_item(&screen_parameters_app, &window) + { + log::warn!( + "Could not reanchor visible menu-bar popover after screen parameters changed: {}", + error + ); + } + }); + let screen_parameters_observer = unsafe { + center.addObserverForName_object_queue_usingBlock( + Some(NSApplicationDidChangeScreenParametersNotification), + None, + None, + &screen_parameters_handler, + ) + }; + + MAC_WINDOW_OBSERVERS.with(|observers| { + observers.borrow_mut().replace(MacWindowObservers { + escape_monitor, + become_active_observer, + resign_observer, + screen_parameters_observer, + }); + }); + Ok(()) +} + +#[cfg(not(target_os = "macos"))] +pub(crate) fn install_macos_event_handlers(_app: &AppHandle) -> Result<(), String> { + Ok(()) +} + +pub(crate) fn acknowledge_close(token: u32) -> bool { + CLOSE_REQUESTS.retire(token) +} + +/// Give the WebView first refusal on a popover close request. +/// +/// A listener acknowledges immediately only when an overlay consumes Escape. +/// Otherwise dismiss_close retires the token after native hiding succeeds. +/// An unhandled, failed or unanswered request gets a one-second fallback when +/// the native event loop can run. Reopening or a newer close invalidates both +/// the old fallback and any delayed dismissal RPC carrying the old token. +pub(crate) fn request_close(app: &AppHandle) -> Result<(), String> { + #[cfg(target_os = "macos")] + if MainThreadMarker::new().is_none() { + let task_app = app.clone(); + return run_on_main_thread_sync(app, move || request_close(&task_app))?; + } + let token = CLOSE_REQUESTS.begin(); + // Arm the fallback before notifying the WebView. An emit failure also gets + // a retry if the immediate native hide fails. + let fallback_app = app.clone(); + std::thread::spawn(move || { + std::thread::sleep(CLOSE_REQUEST_FALLBACK); + if let Err(error) = dismiss_close(&fallback_app, token) { + log::warn!("Failed to dismiss unacknowledged menu-bar popover close request: {error}"); + } + }); + let emit_result = app + .get_webview_window("main") + .ok_or_else(|| "Main window not found".to_string()) + .and_then(|window| { + window + .emit("window:close-requested", CloseRequestedPayload { token }) + .map_err(|error| error.to_string()) + }); + + if let Err(emit_error) = emit_result { + return match dismiss_close(app, token) { + Ok(true) => Err(format!( + "Failed to emit popover close request; dismissed immediately: {emit_error}" + )), + Ok(false) => Err(format!( + "Failed to emit popover close request; no current window was dismissed: {emit_error}" + )), + Err(dismiss_error) => Err(format!( + "Failed to emit popover close request ({emit_error}) and immediate dismissal failed ({dismiss_error})" + )), + }; + } + + Ok(()) +} + +pub(crate) fn show(app: &AppHandle, mode: MainWindowMode) -> Result<(), String> { + #[cfg(target_os = "macos")] + if mode.is_popover() && MainThreadMarker::new().is_none() { + let task_app = app.clone(); + return run_on_main_thread_sync(app, move || show(&task_app, mode))?; + } + // An explicit reopen supersedes any unacknowledged close fallback so an + // older timer can never hide the newly shown popover. + CLOSE_REQUESTS.clear(); + let Some(window) = app.get_webview_window("main") else { + return Ok(()); + }; + let refocus_generation = mode + .is_popover() + .then(|| REFOCUS_GENERATION.fetch_add(1, Ordering::SeqCst) + 1); + + if mode.is_popover() { + window.unminimize().map_err(|error| error.to_string())?; + } + + window.show().map_err(|error| error.to_string())?; + #[cfg(target_os = "macos")] + if mode.is_popover() { + // Explicit dismissal hides the application to return keyboard focus. + // Order the window first so activation observes it as already visible. + app.show().map_err(|error| error.to_string())?; + } + if mode.is_popover() { + // Do not let a launch-time DidBecomeActive notification bypass the + // status-item readiness path. Once any explicit/initial show succeeds, + // later keyboard/programmatic app activation may reopen the popover. + ACTIVATION_REOPEN_ARMED.store(true, Ordering::SeqCst); + } + + // AppKit may replace a hidden window's frame while ordering it onscreen. + // Position only after `show` so the status-item anchor remains final. + if mode.is_popover() + && let Err(error) = position_under_status_item(app, &window) + { + if let Err(fallback_error) = position_in_status_screen_visible_frame(app, &window) { + log::warn!( + "Could not position menu-bar popover under its status item ({error}) or within the status screen's visible frame ({fallback_error}); showing at its current position" + ); + } else { + log::warn!( + "Could not position menu-bar popover under its status item; using the status screen's visible-frame fallback: {error}" + ); + } + } + + window.set_focus().map_err(|error| error.to_string())?; + crate::attention::clear_work(app); + if let Some(generation) = refocus_generation { + if let Err(error) = make_webview_first_responder(&window) { + log::debug!("Initial WebView focus failed: {}", error); + } + schedule_delayed_refocus(app, &window, generation); + crate::peek::record_main_window_shown(app)?; + } + Ok(()) +} + +fn schedule_initial_show( + app: &AppHandle, + mode: MainWindowMode, + startup_generation: u64, +) { + let task_app = app.clone(); + if let Err(error) = app.run_on_main_thread(move || { + if !should_continue_initial_show( + startup_generation, + REFOCUS_GENERATION.load(Ordering::SeqCst), + active(&task_app), + ) { + return; + } + if let Err(error) = show(&task_app, mode) { + log::warn!("Failed to show initial menu-bar popover: {}", error); + } + }) { + log::warn!("Could not schedule initial menu-bar popover: {}", error); + } +} + +pub(crate) fn show_initial_when_ready(app: &AppHandle, mode: MainWindowMode) { + if !mode.is_popover() { + return; + } + + // Page load may finish after the user has already opened or dismissed the + // window. Only untouched startup state may begin the automatic show. + let Some(startup_generation) = claim_initial_show_generation(&REFOCUS_GENERATION) else { + return; + }; + let app = app.clone(); + std::thread::spawn(move || { + let Some(window) = app.get_webview_window("main") else { + return; + }; + let mut last_error = None; + for attempt in 0..INITIAL_POPOVER_POSITION_ATTEMPTS { + if attempt > 0 { + std::thread::sleep(INITIAL_POPOVER_POSITION_RETRY); + } + if !should_continue_initial_show( + startup_generation, + REFOCUS_GENERATION.load(Ordering::SeqCst), + active(&app), + ) { + return; + } + match position_under_status_item(&app, &window) { + Ok(()) => { + schedule_initial_show(&app, mode, startup_generation); + return; + } + Err(error) => last_error = Some(error), + } + } + log::warn!( + "Initial menu-bar popover status item remained unavailable; the status screen visible-frame fallback will be used: {}", + last_error.unwrap_or_else(|| "unknown positioning error".to_string()) + ); + schedule_initial_show(&app, mode, startup_generation); + }); +} + +pub(crate) fn dismiss( + app: &AppHandle, + mode: MainWindowMode, +) -> Result { + if !mode.is_popover() { + return Ok(false); + } + #[cfg(target_os = "macos")] + if MainThreadMarker::new().is_none() { + let task_app = app.clone(); + return run_on_main_thread_sync(app, move || dismiss(&task_app, mode))?; + } + let pending = CLOSE_REQUESTS.pending(); + let hidden = hide_popover(app)?; + if hidden { + // Do not clear a newer request created by a reentrant native callback. + CLOSE_REQUESTS.retire(pending); + } + Ok(hidden) +} + +/// Dismiss only the native close request that is still current when the main +/// thread can execute it. Used by both the WebView response and its fallback. +pub(crate) fn dismiss_close(app: &AppHandle, token: u32) -> Result { + if !active(app).is_popover() { + return Ok(false); + } + #[cfg(target_os = "macos")] + if MainThreadMarker::new().is_none() { + let task_app = app.clone(); + return run_on_main_thread_sync(app, move || dismiss_close(&task_app, token))?; + } + CLOSE_REQUESTS.dismiss_if_current(token, || hide_popover(app)) +} + +fn hide_popover(app: &AppHandle) -> Result { + REFOCUS_GENERATION.fetch_add(1, Ordering::SeqCst); + let Some(window) = app.get_webview_window("main") else { + return Ok(false); + }; + window.hide().map_err(|error| error.to_string())?; + #[cfg(target_os = "macos")] + { + let mtm = MainThreadMarker::new() + .ok_or_else(|| "Application dismissal must run on the main thread".to_string())?; + let application = NSApplication::sharedApplication(mtm); + // Escape, Cmd+W and tray dismissal should return to the previous app. + // On app resignation focus has already moved, so hide only the window. + if application.isActive() { + application.hide(None); + } + } + crate::peek::record_main_window_hidden(app)?; + Ok(true) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn rect(x: f64, y: f64, width: f64, height: f64) -> LogicalRect { + LogicalRect { + x, + y, + width, + height, + } + } + + fn bridgeable_escape() -> EscapeContext { + EscapeContext { + is_popover: true, + is_visible: true, + is_key_window: true, + event_targets_main_window: true, + has_owned_native_dialog: false, + webview_owns_first_responder: false, + } + } + + #[test] + fn popover_is_the_only_enabling_variant() { + assert_eq!(resolve_mode(None, false), MainWindowMode::Control); + assert_eq!( + resolve_mode(Some("control"), false), + MainWindowMode::Control + ); + assert_eq!(resolve_mode(Some("false"), false), MainWindowMode::Control); + assert_eq!( + resolve_mode(Some("unknown"), false), + MainWindowMode::Control + ); + assert_eq!( + resolve_mode(Some("popover"), false), + MainWindowMode::Popover + ); + } + + #[test] + fn transient_popover_mode_suppresses_detached_indicator_windows() { + assert!(MainWindowMode::Control.shows_detached_indicators()); + assert!(!MainWindowMode::Popover.shows_detached_indicators()); + } + + #[test] + fn keyboard_activation_reopens_only_an_armed_hidden_popover() { + assert!(should_reopen_after_activation( + MainWindowMode::Popover, + false, + true, + false, + )); + assert!(!should_reopen_after_activation( + MainWindowMode::Control, + false, + true, + false, + )); + assert!(!should_reopen_after_activation( + MainWindowMode::Popover, + true, + true, + false, + )); + assert!(!should_reopen_after_activation( + MainWindowMode::Popover, + false, + false, + false, + )); + } + + #[test] + fn pointer_activation_stays_owned_by_the_status_item_dock_or_tray_menu() { + assert!(!should_reopen_after_activation( + MainWindowMode::Popover, + false, + true, + true, + )); + } + + #[test] + fn e2e_control_wins_over_a_popover_override() { + assert_eq!(resolve_mode(Some("popover"), true), MainWindowMode::Control); + } + + #[test] + fn an_explicit_e2e_popover_mode_allows_native_mock_qa() { + assert!(should_force_e2e_control(true, None)); + assert!(should_force_e2e_control(true, Some("control"))); + assert!(!should_force_e2e_control(true, Some("popover"))); + assert!(!should_force_e2e_control(false, None)); + assert_eq!( + resolve_e2e_window_mode(Some("popover"), true), + Some(MainWindowMode::Popover) + ); + assert_eq!( + resolve_e2e_window_mode(Some("control"), true), + Some(MainWindowMode::Control) + ); + assert_eq!(resolve_e2e_window_mode(Some("popover"), false), None); + assert_eq!(resolve_e2e_window_mode(Some("unknown"), true), None); + } + + #[test] + fn popover_uses_status_screen_visible_frame_in_logical_coordinates() { + let visible_frame = rect(1_440.0, 25.0, 1_920.0, 1_055.0); + let status_item = rect(3_100.0, 1_056.0, 24.0, 24.0); + assert_eq!( + popover_origin(status_item, 800.0, 600.0, visible_frame), + (2_560.0, 456.0) + ); + } + + #[test] + fn popover_clamps_to_visible_work_area_edges() { + let visible_frame = rect(-1_920.0, 25.0, 1_920.0, 1_055.0); + assert_eq!( + popover_origin( + rect(-1_910.0, 1_056.0, 24.0, 24.0), + 800.0, + 600.0, + visible_frame, + ), + (-1_920.0, 456.0) + ); + assert_eq!( + popover_origin( + rect(-20.0, 1_056.0, 24.0, 24.0), + 800.0, + 600.0, + visible_frame, + ), + (-800.0, 456.0) + ); + + let dock_inset_visible_frame = rect(50.0, 0.0, 2_958.0, 1_662.0); + assert_eq!( + popover_origin( + rect(20.0, 1_662.0, 36.0, 30.0), + 800.0, + 600.0, + dock_inset_visible_frame, + ), + (50.0, 1_062.0) + ); + + let lower_display_visible_frame = rect(0.0, -1_200.0, 1_920.0, 1_170.0); + assert_eq!( + popover_origin( + rect(1_000.0, -30.0, 24.0, 30.0), + 800.0, + 600.0, + lower_display_visible_frame, + ), + (612.0, -630.0) + ); + } + + #[test] + fn popover_visible_frame_fallback_uses_the_top_right_work_area_corner() { + assert_eq!( + popover_visible_frame_fallback_origin( + 800.0, + 600.0, + rect(-1_920.0, 25.0, 1_920.0, 1_055.0), + ), + (-800.0, 480.0), + ); + assert_eq!( + popover_visible_frame_fallback_origin(800.0, 600.0, rect(0.0, 25.0, 700.0, 500.0)), + (0.0, 25.0), + ); + } + + #[test] + fn status_item_frame_must_be_laid_out_before_positioning() { + let screen_frame = rect(0.0, 0.0, 3_008.0, 1_692.0); + assert!(!status_item_frame_is_usable( + rect(0.0, 0.0, 36.0, 0.0), + screen_frame + )); + assert!(status_item_frame_is_usable( + rect(2_063.0, 1_662.0, 36.0, 30.0), + screen_frame + )); + assert!(status_item_frame_is_usable( + rect(2_063.0, 1_670.0, 36.0, 22.0), + screen_frame + )); + assert!(!status_item_frame_is_usable( + rect(0.0, -11.0, 35.5, 22.0), + screen_frame + )); + assert!(!status_item_frame_is_usable( + rect(0.0, 1_662.0, 3_008.0, 30.0), + screen_frame + )); + assert!(!status_item_frame_is_usable( + rect(2_063.0, 1_692.0, 36.0, 30.0), + screen_frame + )); + } + + #[test] + fn webview_owned_escape_passes_through_to_dom() { + let mut context = bridgeable_escape(); + context.webview_owns_first_responder = true; + assert_eq!(escape_action(context), EscapeAction::PassThrough); + } + + #[test] + fn native_dialog_owned_escape_passes_through() { + let mut context = bridgeable_escape(); + context.has_owned_native_dialog = true; + assert_eq!(escape_action(context), EscapeAction::PassThrough); + + context.has_owned_native_dialog = false; + context.event_targets_main_window = false; + assert_eq!(escape_action(context), EscapeAction::PassThrough); + } + + #[test] + fn early_main_window_escape_bridges_to_webview() { + assert_eq!( + escape_action(bridgeable_escape()), + EscapeAction::BridgeToWebview + ); + } + + #[test] + fn delayed_refocus_requires_the_current_visible_popover_generation() { + assert!(should_run_delayed_refocus( + 4, + 4, + MainWindowMode::Popover, + true + )); + assert!(!should_run_delayed_refocus( + 4, + 5, + MainWindowMode::Popover, + true + )); + assert!(!should_run_delayed_refocus( + 4, + 4, + MainWindowMode::Control, + true + )); + assert!(!should_run_delayed_refocus( + 4, + 4, + MainWindowMode::Popover, + false + )); + } + + #[test] + fn initial_show_is_cancelled_by_any_newer_window_action() { + assert!(should_continue_initial_show(4, 4, MainWindowMode::Popover)); + assert!(!should_continue_initial_show(4, 5, MainWindowMode::Popover)); + assert!(!should_continue_initial_show(4, 4, MainWindowMode::Control)); + } + + #[test] + fn late_page_load_cannot_override_an_earlier_window_action() { + let generation = AtomicU64::new(0); + // Explicit show/dismiss before PageLoadEvent::Finished advances this + // same counter, even though the automatic startup show has not begun. + generation.fetch_add(1, Ordering::SeqCst); + assert_eq!(claim_initial_show_generation(&generation), None); + assert_eq!(generation.load(Ordering::SeqCst), 1); + } + + #[test] + fn initial_show_can_claim_only_untouched_startup_state() { + let generation = AtomicU64::new(0); + assert_eq!(claim_initial_show_generation(&generation), Some(1)); + assert_eq!(claim_initial_show_generation(&generation), None); + assert!(should_continue_initial_show( + 1, + generation.load(Ordering::SeqCst), + MainWindowMode::Popover, + )); + + generation.fetch_add(1, Ordering::SeqCst); + assert!(!should_continue_initial_show( + 1, + generation.load(Ordering::SeqCst), + MainWindowMode::Popover, + )); + } + + #[test] + fn screen_changes_only_reanchor_a_visible_popover() { + assert!(should_reanchor_after_screen_change( + MainWindowMode::Popover, + true + )); + assert!(!should_reanchor_after_screen_change( + MainWindowMode::Popover, + false + )); + assert!(!should_reanchor_after_screen_change( + MainWindowMode::Control, + true + )); + } + + #[test] + fn hidden_and_minimized_tray_clicks_show_the_popover() { + for (is_visible, is_minimized) in [(false, false), (true, true)] { + let down = decide_tray_click( + MainWindowMode::Popover, + TrayClickState::default(), + TrayClickInput::Down { + is_visible, + is_minimized, + }, + ); + assert_eq!(down.action, TrayClickAction::None); + let up = decide_tray_click(MainWindowMode::Popover, down.state, TrayClickInput::Up); + assert_eq!(up.action, TrayClickAction::Show); + assert_eq!(up.state, TrayClickState::default()); + } + } + + #[test] + fn visible_tray_click_dismisses_and_resets_for_the_next_gesture() { + let down = decide_tray_click( + MainWindowMode::Popover, + TrayClickState::default(), + TrayClickInput::Down { + is_visible: true, + is_minimized: false, + }, + ); + let up = decide_tray_click(MainWindowMode::Popover, down.state, TrayClickInput::Up); + assert_eq!(up.action, TrayClickAction::Dismiss); + assert_eq!(up.state, TrayClickState::default()); + + let next_up = decide_tray_click(MainWindowMode::Popover, up.state, TrayClickInput::Up); + assert_eq!(next_up.action, TrayClickAction::Show); + } + + #[test] + fn control_mode_ignores_and_resets_popover_tray_state() { + let decision = decide_tray_click( + MainWindowMode::Control, + TrayClickState { + started_visible: true, + }, + TrayClickInput::Up, + ); + assert_eq!(decision.action, TrayClickAction::None); + assert_eq!(decision.state, TrayClickState::default()); + } + + #[test] + fn close_request_in_time_ack_cancels_its_fallback() { + assert_eq!(CLOSE_REQUEST_FALLBACK, Duration::from_secs(1)); + let requests = CloseRequestTracker::new(); + let token = requests.begin(); + assert!(requests.retire(token)); + assert!( + !requests + .dismiss_if_current(token, || panic!("acknowledged fallback must not hide")) + .unwrap() + ); + } + + #[test] + fn missing_close_listener_allows_timeout_and_rejects_late_ack() { + let requests = CloseRequestTracker::new(); + let token = requests.begin(); + assert!(requests.dismiss_if_current(token, || Ok(true)).unwrap()); + assert!(!requests.retire(token), "late ack must be a no-op"); + } + + #[test] + fn close_is_pending_during_hide_and_retires_only_after_success() { + let requests = CloseRequestTracker::new(); + let token = requests.begin(); + assert!( + requests + .dismiss_if_current(token, || { + assert_eq!(requests.pending(), token); + Ok(true) + }) + .unwrap() + ); + assert_eq!(requests.pending(), 0); + assert!( + !requests + .dismiss_if_current(token, || panic!("fallback must not hide twice")) + .unwrap() + ); + } + + #[test] + fn failed_hide_leaves_close_pending_for_native_fallback() { + let requests = CloseRequestTracker::new(); + let token = requests.begin(); + assert_eq!( + requests.dismiss_if_current(token, || Err("native hide failed".into())), + Err("native hide failed".into()) + ); + assert_eq!(requests.pending(), token); + assert!(requests.dismiss_if_current(token, || Ok(true)).unwrap()); + assert_eq!(requests.pending(), 0); + } + + #[test] + fn unsuccessful_hide_does_not_consume_the_close_fallback() { + let requests = CloseRequestTracker::new(); + let token = requests.begin(); + assert!(!requests.dismiss_if_current(token, || Ok(false)).unwrap()); + assert_eq!(requests.pending(), token); + assert!(requests.dismiss_if_current(token, || Ok(true)).unwrap()); + } + + #[test] + fn reopen_invalidates_a_delayed_dismissal_before_native_hide() { + let requests = CloseRequestTracker::new(); + let token = requests.begin(); + requests.clear(); + assert!( + !requests + .dismiss_if_current(token, || panic!("stale RPC must not hide reopened window")) + .unwrap() + ); + } + + #[test] + fn stale_dismissal_cannot_hide_or_retire_a_newer_close() { + let requests = CloseRequestTracker::new(); + let old_token = requests.begin(); + let current = requests.begin(); + assert!( + !requests + .dismiss_if_current(old_token, || panic!("stale dismissal must not run")) + .unwrap() + ); + assert_eq!(requests.pending(), current); + assert!(requests.dismiss_if_current(current, || Ok(true)).unwrap()); + } + + #[test] + fn successful_hide_does_not_clear_a_reentrant_newer_close() { + let requests = CloseRequestTracker::new(); + let token = requests.begin(); + let mut newer = 0; + assert!( + requests + .dismiss_if_current(token, || { + newer = requests.begin(); + Ok(true) + }) + .unwrap() + ); + assert_ne!(token, newer); + assert_eq!(requests.pending(), newer); + } + + #[test] + fn close_tokens_wrap_without_reusing_the_empty_sentinel() { + let requests = CloseRequestTracker::new(); + requests.next_token.store(u32::MAX, Ordering::SeqCst); + assert_eq!(requests.begin(), u32::MAX); + let wrapped = requests.begin(); + assert_eq!(wrapped, 1); + assert!(!requests.retire(u32::MAX)); + assert!(requests.retire(wrapped)); + } + + #[test] + fn newest_close_request_wins_over_stale_ack_and_older_timeout() { + let requests = CloseRequestTracker::new(); + let older = requests.begin(); + let current = requests.begin(); + assert!(current > older); + assert!(!requests.retire(older), "stale ack must be a no-op"); + assert!(!requests.retire(older), "older timeout must be a no-op"); + assert!(requests.retire(current)); + } +} diff --git a/apps/native/src-tauri/src/orpc/darwin.rs b/apps/native/src-tauri/src/orpc/darwin.rs index 6e02e110c..57ff4225c 100644 --- a/apps/native/src-tauri/src/orpc/darwin.rs +++ b/apps/native/src-tauri/src/orpc/darwin.rs @@ -8,7 +8,7 @@ use crate::privileged_helper::{ }; use crate::shared_types::{ AppManagementCheckResult, BuildCheckResult, EtcClobberCheckResult, EvolveCancelResult, - OkResult, RebuildStatus, RollbackResult, + HelperPermissionPhase, OkResult, RebuildStatus, RollbackResult, }; use crate::system::helper_permission; use orpc::*; @@ -75,6 +75,7 @@ struct InstallSyncAgentInput { #[serde(rename_all = "camelCase")] struct HelperReport { at_this_build: bool, + phase: HelperPermissionPhase, detail: String, } @@ -85,7 +86,8 @@ impl HelperReport { report, crate::privileged_helper::reconcile::Reconciled::AtThisBuild ), - detail: helper_permission::describe(report), + phase: helper_permission::phase(report), + detail: helper_permission::action_detail(report), } } } @@ -233,6 +235,12 @@ async fn helper_grant(ctx: OrpcCtx, _input: ()) -> Result Result { + Ok(HelperReport::of(&helper_permission::retry(&ctx.app))) +} + /// The explicit Disable action: unregister the helper (deferring while an /// activation runs) and /// register nothing. No later automatic run overrides it. @@ -318,6 +326,9 @@ pub fn routes() -> Router { "helperGrant" => os::() .output(orpc_specta::specta::()) .handler(helper_grant), + "helperRetry" => os::() + .output(orpc_specta::specta::()) + .handler(helper_retry), "helperDisable" => os::() .output(orpc_specta::specta::()) .handler(helper_disable), diff --git a/apps/native/src-tauri/src/orpc/main_window.rs b/apps/native/src-tauri/src/orpc/main_window.rs new file mode 100644 index 000000000..947200dda --- /dev/null +++ b/apps/native/src-tauri/src/orpc/main_window.rs @@ -0,0 +1,50 @@ +//! Main-window mode and popover lifecycle procedures. + +use super::{OrpcCtx, helpers::internal_err}; +use crate::main_window; +use orpc::*; +use serde::{Deserialize, Serialize}; +use specta::Type; + +#[derive(Debug, Deserialize, Serialize, Type)] +#[serde(rename_all = "camelCase")] +struct CloseRequestInput { + token: u32, +} + +async fn is_popover(ctx: OrpcCtx, _input: ()) -> Result { + Ok(main_window::active(&ctx.app).is_popover()) +} + +async fn dismiss_popover(ctx: OrpcCtx, _input: ()) -> Result { + main_window::dismiss(&ctx.app, main_window::active(&ctx.app)) + .map_err(|error| internal_err("mainWindow.dismissPopover", error)) +} + +async fn dismiss_close(ctx: OrpcCtx, input: CloseRequestInput) -> Result { + main_window::dismiss_close(&ctx.app, input.token) + .map_err(|error| internal_err("mainWindow.dismissClose", error)) +} + +async fn acknowledge_close(_ctx: OrpcCtx, input: CloseRequestInput) -> Result { + Ok(main_window::acknowledge_close(input.token)) +} + +pub fn routes() -> Router { + router! { + "isPopover" => os::() + .output(orpc_specta::specta::()) + .handler(is_popover), + "dismissPopover" => os::() + .output(orpc_specta::specta::()) + .handler(dismiss_popover), + "dismissClose" => os::() + .input(orpc_specta::specta::()) + .output(orpc_specta::specta::()) + .handler(dismiss_close), + "acknowledgeClose" => os::() + .input(orpc_specta::specta::()) + .output(orpc_specta::specta::()) + .handler(acknowledge_close), + } +} diff --git a/apps/native/src-tauri/src/orpc/mod.rs b/apps/native/src-tauri/src/orpc/mod.rs index 6da2cb5dd..ebf773d94 100644 --- a/apps/native/src-tauri/src/orpc/mod.rs +++ b/apps/native/src-tauri/src/orpc/mod.rs @@ -21,6 +21,7 @@ mod history; mod homebrew; mod launchd; mod lsp; +mod main_window; mod models; mod nix; mod onboarding; @@ -56,6 +57,7 @@ pub fn build_router() -> Router { .nest("summarizedChanges", summarized_changes::routes()) .nest("history", history::routes()) .nest("git", git::routes()) + .nest("mainWindow", main_window::routes()) .nest("previewIndicator", preview_indicator::routes()) .nest("homebrew", homebrew::routes()) .nest("launchd", launchd::routes()) diff --git a/apps/native/src-tauri/src/panic_handler.rs b/apps/native/src-tauri/src/panic_handler.rs index 03f0b5eff..ce3946224 100644 --- a/apps/native/src-tauri/src/panic_handler.rs +++ b/apps/native/src-tauri/src/panic_handler.rs @@ -15,7 +15,7 @@ use log::error; use std::panic; use tauri::{AppHandle, Emitter, Manager}; -use crate::shared_types::RustPanicEvent; +use crate::{main_window, shared_types::RustPanicEvent}; /// Sets up the custom panic hook for the application. pub fn setup_panic_hook(app_handle: AppHandle) { @@ -71,6 +71,8 @@ pub fn setup_panic_hook(app_handle: AppHandle) { timestamp, }; + let mode = main_window::active(&app_handle); + // Try to emit event to frontend if let Some(window) = app_handle.get_webview_window("main") { if let Err(e) = window.emit("rust:panic", &panic_payload) { @@ -82,6 +84,12 @@ pub fn setup_panic_hook(app_handle: AppHandle) { error!("❌ Could not get main window to emit panic event"); } + // Recoverable command panics leave the process alive, so a hidden popover can + // wait for the notification click. Fatal panics may terminate before delivery. + if mode.is_popover() { + crate::attention::crash(&app_handle); + } + // Call the default hook to let Rust proceed with its normal panic unwinding. // If this panic happened in a Tauri command handler, the `#[tauri::command]` wrapper // will catch the unwind and return an error response. Otherwise, the process will terminate. diff --git a/apps/native/src-tauri/src/peek.rs b/apps/native/src-tauri/src/peek.rs index 63b430c41..6c64aa9df 100644 --- a/apps/native/src-tauri/src/peek.rs +++ b/apps/native/src-tauri/src/peek.rs @@ -245,19 +245,7 @@ pub fn show_main_window(app: &AppHandle) -> Result<(), String> { peek_log!("📍 Showing main window"); window.show().map_err(|e| e.to_string())?; window.set_focus().map_err(|e| e.to_string())?; - - MAIN_WINDOW_OPEN.store(true, Ordering::SeqCst); - - // Hide the icon - hide_icon(app)?; - - // Note: Preview indicator stays visible when there are changes, - // even when main window is open - - // Emit state change - let _ = window.emit("peek:state", PeekState::Expanded); - - Ok(()) + record_main_window_shown(app) } /// Hides the main window @@ -265,13 +253,31 @@ pub fn hide_main_window(app: &AppHandle) -> Result<(), String> { if let Some(window) = app.get_webview_window("main") { window.hide().map_err(|e| e.to_string())?; peek_log!("🙈 Hiding main window"); - MAIN_WINDOW_OPEN.store(false, Ordering::SeqCst); - let _ = window.emit("peek:state", PeekState::Hidden); + record_main_window_hidden(app)?; + } + Ok(()) +} - // Show preview indicator if there are uncommitted changes - if HAS_UNCOMMITTED_CHANGES.load(Ordering::SeqCst) { - show_preview_indicator(app)?; - } +/// Keeps the shared peek state in sync after any main-window show path. +pub(crate) fn record_main_window_shown(app: &AppHandle) -> Result<(), String> { + MAIN_WINDOW_OPEN.store(true, Ordering::SeqCst); + let hide_icon_result = hide_icon(app); + if let Some(window) = app.get_webview_window("main") { + let _ = window.emit("peek:state", PeekState::Expanded); + } + hide_icon_result +} + +/// Keeps the shared peek state and preview indicator in sync after any hide path. +pub(crate) fn record_main_window_hidden(app: &AppHandle) -> Result<(), String> { + MAIN_WINDOW_OPEN.store(false, Ordering::SeqCst); + if let Some(window) = app.get_webview_window("main") { + let _ = window.emit("peek:state", PeekState::Hidden); + } + if crate::main_window::active(app).shows_detached_indicators() + && HAS_UNCOMMITTED_CHANGES.load(Ordering::SeqCst) + { + show_preview_indicator(app)?; } Ok(()) } @@ -568,7 +574,9 @@ pub fn create_preview_indicator_window(app: &AppHandle) -> Result /// Shows the preview indicator window pub fn show_preview_indicator(app: &AppHandle) -> Result<(), String> { - if !get_preview_indicator_state().visible { + if !crate::main_window::active(app).shows_detached_indicators() + || !get_preview_indicator_state().visible + { // The transparent webview would block the underlying application even // though React renders no indicator. Keep it hidden in that case. return hide_preview_indicator(app); @@ -646,7 +654,9 @@ pub fn update_preview_indicator( // Show preview indicator whenever there are visible changes, // regardless of whether main window is open - if state.visible { + if !crate::main_window::active(app).shows_detached_indicators() { + hide_preview_indicator(app)?; + } else if state.visible { show_preview_indicator(app)?; } else { hide_preview_indicator(app)?; diff --git a/apps/native/src-tauri/src/privileged_helper/protocol.rs b/apps/native/src-tauri/src/privileged_helper/protocol.rs index c8a516cce..db4322fb7 100644 --- a/apps/native/src-tauri/src/privileged_helper/protocol.rs +++ b/apps/native/src-tauri/src/privileged_helper/protocol.rs @@ -11,7 +11,7 @@ use std::path::{Component, Path, PathBuf}; #[cfg_attr(not(test), allow(dead_code))] pub const HELPER_LABEL: &str = "com.darkmatter.nixmac.helper"; pub const SYNC_AGENT_LABEL: &str = "com.darkmatter.nixmac.sync-agent"; -#[cfg(target_os = "macos")] +// Bundle definition validation runs on every target; only registration is macOS-only. pub const HELPER_PLIST_NAME: &str = "com.darkmatter.nixmac.helper.plist"; #[allow(dead_code)] pub const SYNC_AGENT_PLIST_NAME: &str = "com.darkmatter.nixmac.sync-agent.plist"; diff --git a/apps/native/src-tauri/src/privileged_helper/reconcile.rs b/apps/native/src-tauri/src/privileged_helper/reconcile.rs index 7c61a6ec1..053726220 100644 --- a/apps/native/src-tauri/src/privileged_helper/reconcile.rs +++ b/apps/native/src-tauri/src/privileged_helper/reconcile.rs @@ -37,7 +37,7 @@ use crate::privileged_helper::client::{ }; use crate::privileged_helper::protocol::{self, ActivationInfo, HelperReply, HelperStateName}; use crate::privileged_helper::service::{ - self, RegisterFailure, RegistrationStatus, ReplaceFailure, + self, RegisterFailure, RegistrationStatus, ReplaceFailure, ServiceDefinitionError, }; use crate::shared_types::{HelperDecision, HelperPreference}; use crate::state::preferences; @@ -118,7 +118,7 @@ pub enum Reconciled { /// was unregistered, registered, or written, and no later run changes the /// answer: the user has to move the app or restart it. Displaced(String), - /// Final: `notFound` — the bundle's own service definition is broken, so + /// Final: the bundle's own service definition is missing or invalid, so /// there is nothing to register and nothing to remove. A property of the /// bundle, read identically every time. ServiceDefinitionBroken, @@ -256,6 +256,10 @@ pub trait Environment { /// What `SMAppService` reports about nixmac's helper registration. fn registration_status(&self) -> Result; + /// Inspect the bundled plist and executable independently of the framework + /// record, which may not exist before the first registration. + fn check_service_definition(&self) -> Result<(), ServiceDefinitionError>; + /// The stored decision about the helper. fn preference(&self) -> Result; @@ -313,6 +317,15 @@ impl Environment for LiveEnvironment<'_, R> { service::registration_status().map_err(|error| format!("{error:#}")) } + fn check_service_definition(&self) -> Result<(), ServiceDefinitionError> { + match install_location::locate_app_bundle() { + InstallLocation::Canonical(bundle) => service::check_service_definition(&bundle), + InstallLocation::Elsewhere(_) => Err(ServiceDefinitionError::Unavailable( + "the installed app bundle could not be located".to_string(), + )), + } + } + fn preference(&self) -> Result { preferences::read_helper_preference(self.app).map_err(|error| format!("{error:#}")) } @@ -439,6 +452,13 @@ pub fn grant(env: &E) -> Reconciled { decide(&RUNNING, env, HelperDecision::Granted) } +/// Explicitly retry the existing standing decision. Unlike a routine +/// observation this waits for the single-flight slot, but unlike [`grant`] it +/// never changes an opt-in or opens System Settings. +pub fn retry(env: &E) -> Reconciled { + retry_with(&RUNNING, env) +} + /// The explicit Disable action: record the decision, then reconcile under it. /// The decision is written first, so a crash halfway resumes the removal /// rather than repairing the helper. @@ -468,6 +488,11 @@ fn decide(running: &Mutex<()>, env: &E, decision: HelperDecision reported(converge(env).unwrap_or_else(|report| report)) } +fn retry_with(running: &Mutex<()>, env: &E) -> Reconciled { + let _slot = running.lock().unwrap_or_else(PoisonError::into_inner); + reported(converge(env).unwrap_or_else(|report| report)) +} + fn run(running: &Mutex<()>, env: &E) -> Reconciled { let _slot = match running.try_lock() { Ok(slot) => slot, @@ -497,10 +522,19 @@ enum Goal { fn converge(env: &E) -> Step { env.gate()?; let status = status(env)?; + if status == RegistrationStatus::NotFound { + // On a first install BTM can read a valid daemon plist but have no + // record for it yet. Validate the package before deciding from opt-in. + check_definition(env)?; + } let goal = goal(env, status)?; match (status, goal) { - (RegistrationStatus::NotRegistered, Goal::Install) => register_fresh(env), - (RegistrationStatus::NotRegistered, Goal::Remove) => Ok(Reconciled::NoHelper), + (RegistrationStatus::NotRegistered | RegistrationStatus::NotFound, Goal::Install) => { + register_fresh(env) + } + (RegistrationStatus::NotRegistered | RegistrationStatus::NotFound, Goal::Remove) => { + Ok(Reconciled::NoHelper) + } // Approval is a state, not an error: reported, never re-registered, // and never a reason to open System Settings. This arm registers // nothing, however often it is reached. @@ -509,12 +543,33 @@ fn converge(env: &E) -> Step { // to observe before removing it. (RegistrationStatus::RequiresApproval, Goal::Remove) => remove(env), (RegistrationStatus::Enabled, goal) => classify(env, goal), - // Nothing to register and nothing to remove: the definition this - // would act on is not there. - (RegistrationStatus::NotFound, _) => Err(Reconciled::ServiceDefinitionBroken), } } +fn check_definition(env: &E) -> Result<(), Reconciled> { + env.check_service_definition().map_err(|error| match error { + ServiceDefinitionError::Invalid(detail) => { + log::warn!("invalid bundled helper definition: {detail}"); + Reconciled::ServiceDefinitionBroken + } + ServiceDefinitionError::Unavailable(detail) => Reconciled::Stopped(format!( + "the bundled helper definition could not be checked: {detail}" + )), + }) +} + +/// Missing framework state after a register is a failed attempt, not proof +/// that a valid local package is broken. A later explicit/bounded retry can +/// try again, and readiness still requires the authenticated socket exchange. +fn missing_registered_service(env: &E, register_error: Option<&str>) -> Step { + check_definition(env)?; + let missing = "ServiceManagement has no helper record after registration"; + Err(Reconciled::Stopped(match register_error { + Some(error) => format!("{error}; {missing}"), + None => missing.to_string(), + })) +} + fn status(env: &E) -> Result { env.registration_status().map_err(|detail| { Reconciled::Stopped(format!( @@ -546,9 +601,9 @@ fn goal(env: &E, status: RegistrationStatus) -> Result Err(Reconciled::NoHelper), - // Never resolves the decision; reported as the failure it is. - RegistrationStatus::NotFound => Err(Reconciled::ServiceDefinitionBroken), + RegistrationStatus::NotRegistered | RegistrationStatus::NotFound => { + Err(Reconciled::NoHelper) + } }, } } @@ -689,20 +744,24 @@ fn remove(env: &E) -> Step { fn replace(env: &E) -> Step { match env.replace_helper(&|| commit_to_register(env)) { Ok(()) => verify(env), - Err(failure) => Err(match failure { + Err(failure) => match failure { // The register step's own report, whatever it was: this pass // unregistered the old helper and deliberately registered // nothing in its place. - ReplaceFailure::RegisterDeclined(report) => report, - ReplaceFailure::UnregisterFailed(error) => unregister_failed(error), - ReplaceFailure::UnregisterSilent => unregister_failed("the unregister never reported"), - ReplaceFailure::RegisterFailed(error) => register_failed(error), - ReplaceFailure::RegisterSilent => register_failed("the register never reported"), - // Refused before anything was dispatched, so nothing changed. - ReplaceFailure::CalledOnMainThread => { - register_failed("a helper replacement cannot run on the main thread") + ReplaceFailure::RegisterDeclined(report) => Err(report), + ReplaceFailure::UnregisterFailed(error) => Err(unregister_failed(error)), + ReplaceFailure::UnregisterSilent => { + Err(unregister_failed("the unregister never reported")) } - }), + ReplaceFailure::RegisterFailed(error) => after_register_failure(env, error.to_string()), + ReplaceFailure::RegisterSilent => { + after_register_failure(env, "the register never reported".to_string()) + } + // Refused before anything was dispatched, so nothing changed. + ReplaceFailure::CalledOnMainThread => Err(register_failed( + "a helper replacement cannot run on the main thread", + )), + }, } } @@ -710,14 +769,38 @@ fn register_fresh(env: &E) -> Step { // The gates are inside the commitment, which is where every register's // immediately-before checks live. let committed = commit_to_register(env)?; - env.register(committed).map_err(|failure| match failure { - RegisterFailure::Failed(error) => register_failed(error), - RegisterFailure::Silent => register_failed("the register never reported"), - RegisterFailure::CalledOnMainThread => { - register_failed("a registration cannot run on the main thread") + match env.register(committed) { + Ok(()) => verify(env), + Err(RegisterFailure::Failed(error)) => after_register_failure(env, error.to_string()), + Err(RegisterFailure::Silent) => { + after_register_failure(env, "the register never reported".to_string()) } - })?; - verify(env) + Err(RegisterFailure::CalledOnMainThread) => Err(register_failed( + "a registration cannot run on the main thread", + )), + } +} + +/// A ServiceManagement register may report an error after macOS has already +/// moved the service into a meaningful state. Judge that state once, without +/// keying behavior to an unstable numeric error code or issuing a second +/// register in this pass. +fn after_register_failure(env: &E, register_error: String) -> Step { + log::warn!("helper register reported an error: {register_error}"); + match env.registration_status() { + Ok(RegistrationStatus::RequiresApproval) => Ok(Reconciled::PendingApproval), + Ok(RegistrationStatus::Enabled) => verify_listening(env).map_err(|report| match report { + Reconciled::Stopped(detail) => Reconciled::Stopped(format!( + "{register_error}; helper verification failed: {detail}" + )), + other => other, + }), + Ok(RegistrationStatus::NotRegistered) => Err(register_failed(register_error)), + Ok(RegistrationStatus::NotFound) => missing_registered_service(env, Some(®ister_error)), + Err(status_error) => Err(register_failed(format!( + "{register_error}; the resulting registration status could not be read: {status_error}" + ))), + } } /// The register step's immediately-before checks, and the only mint of the @@ -737,6 +820,7 @@ fn register_fresh(env: &E) -> Step { fn commit_to_register(env: &E) -> Result { // A register is a mutation like any other. env.gate()?; + check_definition(env)?; match env.preference() { Ok(HelperPreference::Granted) => Ok(evidence::Committed::checked()), Ok(HelperPreference::Disabled | HelperPreference::Unset) => Err(Reconciled::Stopped( @@ -750,25 +834,26 @@ fn commit_to_register(env: &E) -> Result fn verify(env: &E) -> Step { match status(env)? { RegistrationStatus::Enabled => verify_listening(env), - settled => verify_from_status(settled), + settled => verify_from_status(env, settled), } } -/// Judges a dispatched registration from a status read alone. +/// Judges a dispatched registration from fresh framework state. A missing +/// record also checks the local definition before claiming the bundle is broken. /// /// Both status reads a verification can make land here: the one above, whose /// `enabled` goes to the listen window rather than to a verdict, and the fresh /// one taken when that window expires. So the `enabled` arm belongs to the /// second read alone — nothing answered, and the registration still says /// something should have. -fn verify_from_status(status: RegistrationStatus) -> Step { +fn verify_from_status(env: &E, status: RegistrationStatus) -> Step { match status { // Registered, and macOS wants the user's approval. Normal. RegistrationStatus::RequiresApproval => Ok(Reconciled::PendingApproval), RegistrationStatus::NotRegistered => Err(Reconciled::Stopped( "the helper registration disappeared while it was being verified".to_string(), )), - RegistrationStatus::NotFound => Err(Reconciled::ServiceDefinitionBroken), + RegistrationStatus::NotFound => missing_registered_service(env, None), RegistrationStatus::Enabled => Err(Reconciled::Stopped( "the registered helper never answered on its socket".to_string(), )), @@ -805,7 +890,7 @@ fn verify_listening(env: &E) -> Step { // never answered — a failure — for a state that is not one. The window // is the only thing between the two reads, and it is long enough for // macOS to have changed its answer inside it. - verify_from_status(status(env)?) + verify_from_status(env, status(env)?) } fn judge(env: &E, reply: HelperReply) -> Step { @@ -854,6 +939,7 @@ fn exchange_failed(error: HelperClientError) -> Reconciled { mod tests { use super::*; use crate::privileged_helper::peer_auth::ClientKind; + use crate::privileged_helper::service::ServiceCallError; use std::cell::RefCell; use std::path::PathBuf; @@ -937,6 +1023,7 @@ mod tests { /// *which* gate a refusal stopped. gates: RefCell>>, status: RefCell>>, + definition: RefCell>>, preference: RefCell>, exchanges: RefCell>, listener: ListenerObservation, @@ -955,6 +1042,7 @@ mod tests { compiled: THIS_BUILD, gates: RefCell::new(vec![Ok(())]), status: RefCell::new(vec![Ok(RegistrationStatus::Enabled)]), + definition: RefCell::new(vec![Ok(())]), preference: RefCell::new(Ok(HelperPreference::Granted)), exchanges: RefCell::new(Vec::new()), listener: ListenerObservation::PositivelyAbsent, @@ -1024,6 +1112,15 @@ mod tests { } } + fn check_service_definition(&self) -> Result<(), ServiceDefinitionError> { + let mut definitions = self.definition.borrow_mut(); + if definitions.len() > 1 { + definitions.remove(0) + } else { + definitions[0].clone() + } + } + fn preference(&self) -> Result { self.preference.borrow().clone() } @@ -1094,6 +1191,14 @@ mod tests { status_reply(THIS_BUILD, HelperStateName::Idle, None) } + fn register_refusal() -> ServiceCallError { + ServiceCallError { + domain: "SMAppServiceErrorDomain".to_string(), + code: 1, + localized: "operation not permitted".to_string(), + } + } + /// The sentence a stopped report carries, or a failure naming what came /// instead. fn stopped_text(report: &Reconciled) -> String { @@ -1401,14 +1506,358 @@ mod tests { fn a_broken_service_definition_stops_the_pass() { let world = World { status: RefCell::new(vec![Ok(RegistrationStatus::NotFound)]), + definition: RefCell::new(vec![Err(ServiceDefinitionError::Invalid( + "missing helper plist".to_string(), + ))]), ..World::default() }; assert_eq!(fresh_run(&world), Reconciled::ServiceDefinitionBroken); + assert!(world.acts().is_empty()); + } + + #[test] + fn a_valid_never_registered_helper_registers_only_when_granted_and_verifies() { + let world = World::default() + .with_statuses(vec![ + Ok(RegistrationStatus::NotFound), + Ok(RegistrationStatus::Enabled), + ]) + .with_exchanges(vec![Peer::Answered(this_build_idle())]); + + assert_eq!(fresh_run(&world), Reconciled::AtThisBuild); + assert_eq!(world.acts(), vec![Act::Registered]); + } + + #[test] + fn a_valid_never_registered_helper_does_not_adopt_or_override_a_missing_opt_in() { + for preference in [HelperPreference::Unset, HelperPreference::Disabled] { + let world = World { + preference: RefCell::new(Ok(preference)), + status: RefCell::new(vec![Ok(RegistrationStatus::NotFound)]), + ..World::default() + }; + + assert_eq!(fresh_run(&world), Reconciled::NoHelper); + assert!(world.acts().is_empty()); + } + } + + #[test] + fn a_definition_that_becomes_invalid_before_registration_cannot_register() { + let world = World { + status: RefCell::new(vec![Ok(RegistrationStatus::NotFound)]), + definition: RefCell::new(vec![ + Ok(()), + Err(ServiceDefinitionError::Invalid( + "helper executable disappeared".to_string(), + )), + ]), + ..World::default() + }; + + assert_eq!(fresh_run(&world), Reconciled::ServiceDefinitionBroken); + assert!(world.acts().is_empty()); + } + + #[test] + fn an_unreadable_definition_is_a_retryable_failure_and_authorizes_nothing() { + let world = World { + status: RefCell::new(vec![Ok(RegistrationStatus::NotFound)]), + definition: RefCell::new(vec![Err(ServiceDefinitionError::Unavailable( + "permission denied reading helper plist".to_string(), + ))]), + ..World::default() + }; + + assert!(stopped_text(&fresh_run(&world)).contains("could not be checked")); + assert!(world.acts().is_empty()); + } + + #[test] + fn a_missing_framework_record_after_register_is_not_readiness_or_a_broken_bundle() { + let world = World::default().with_statuses(vec![Ok(RegistrationStatus::NotFound)]); + + assert_eq!( + fresh_run(&world), + Reconciled::Stopped( + "ServiceManagement has no helper record after registration".to_string() + ) + ); + assert_eq!( + world.acts(), + vec![Act::Registered], + "only one attempt in the pass" + ); + } + + #[test] + fn a_failed_first_registration_can_retry_and_reach_authenticated_readiness() { + let world = World { + status: RefCell::new(vec![ + Ok(RegistrationStatus::NotFound), + Ok(RegistrationStatus::NotFound), + Ok(RegistrationStatus::NotFound), + Ok(RegistrationStatus::Enabled), + ]), + register: RefCell::new(Some(RegisterFailure::Failed(register_refusal()))), + exchanges: RefCell::new(vec![Peer::Answered(this_build_idle())]), + ..World::default() + }; + + assert!(matches!(fresh_run(&world), Reconciled::Stopped(_))); + assert_eq!(world.acts(), vec![Act::Registered]); + assert_eq!(fresh_run(&world), Reconciled::AtThisBuild); + assert_eq!(world.acts(), vec![Act::Registered, Act::Registered]); } // ── refusals and failures that end a pass ────────────────────────────── + #[test] + fn a_fresh_register_error_is_judged_from_one_authoritative_status_read() { + for (resulting_status, expected) in [ + ( + Ok(RegistrationStatus::RequiresApproval), + Reconciled::PendingApproval, + ), + ( + Ok(RegistrationStatus::NotFound), + Reconciled::Stopped(format!( + "{}; ServiceManagement has no helper record after registration", + register_refusal() + )), + ), + ] { + let world = World { + status: RefCell::new(vec![ + Ok(RegistrationStatus::NotRegistered), + resulting_status, + ]), + register: RefCell::new(Some(RegisterFailure::Failed(register_refusal()))), + ..World::default() + }; + + assert_eq!(fresh_run(&world), expected); + assert_eq!(world.acts(), vec![Act::Registered]); + } + + let world = World { + status: RefCell::new(vec![ + Ok(RegistrationStatus::NotRegistered), + Ok(RegistrationStatus::Enabled), + ]), + register: RefCell::new(Some(RegisterFailure::Failed(register_refusal()))), + exchanges: RefCell::new(vec![Peer::Answered(this_build_idle())]), + ..World::default() + }; + assert_eq!(fresh_run(&world), Reconciled::AtThisBuild); + assert_eq!(world.acts(), vec![Act::Registered]); + } + + #[test] + fn a_fresh_register_error_keeps_its_diagnostics_when_nothing_registered() { + for resulting_status in [ + Ok(RegistrationStatus::NotRegistered), + Err("status unavailable".to_string()), + ] { + let status_was_unavailable = resulting_status.is_err(); + let world = World { + status: RefCell::new(vec![ + Ok(RegistrationStatus::NotRegistered), + resulting_status, + ]), + register: RefCell::new(Some(RegisterFailure::Failed(register_refusal()))), + ..World::default() + }; + + let detail = stopped_text(&fresh_run(&world)); + assert!(detail.contains("SMAppServiceErrorDomain 1")); + if status_was_unavailable { + assert!(detail.contains("status unavailable")); + assert!(detail.contains("resulting registration status could not be read")); + } + assert_eq!(world.acts(), vec![Act::Registered]); + } + } + + #[test] + fn register_errors_survive_failed_verification_for_fresh_and_replacement_helpers() { + for (peers, verification_detail) in [ + ( + vec![Peer::RootUnverifiable], + "the helper socket is held by something unidentified: ad-hoc signature".to_string(), + ), + ( + vec![Peer::Answered(status_reply( + OTHER_BUILD, + HelperStateName::Idle, + None, + ))], + format!("the registered helper reports build {OTHER_BUILD}, not this build"), + ), + ( + vec![Peer::Unreachable; VERIFY_LISTEN_ATTEMPTS as usize], + "the registered helper never answered on its socket".to_string(), + ), + ] { + for replacing in [false, true] { + let mut exchanges = Vec::new(); + if replacing { + exchanges.push(Peer::Answered(status_reply( + OTHER_BUILD, + HelperStateName::Idle, + None, + ))); + } + exchanges.extend(peers.clone()); + let world = World { + status: RefCell::new(vec![ + Ok(if replacing { + RegistrationStatus::Enabled + } else { + RegistrationStatus::NotRegistered + }), + Ok(RegistrationStatus::Enabled), + ]), + register: RefCell::new( + (!replacing).then(|| RegisterFailure::Failed(register_refusal())), + ), + replace: RefCell::new( + replacing.then(|| ReplaceFailure::RegisterFailed(register_refusal())), + ), + exchanges: RefCell::new(exchanges), + ..World::default() + }; + + assert_eq!( + stopped_text(&fresh_run(&world)), + format!( + "{}; helper verification failed: {verification_detail}", + register_refusal() + ) + ); + assert_eq!( + world.acts(), + vec![if replacing { + Act::Replaced + } else { + Act::Registered + }] + ); + assert_eq!(*world.preference.borrow(), Ok(HelperPreference::Granted)); + } + } + } + + #[test] + fn register_error_does_not_change_approval_discovered_during_verification() { + let world = World { + status: RefCell::new(vec![ + Ok(RegistrationStatus::NotRegistered), + Ok(RegistrationStatus::Enabled), + Ok(RegistrationStatus::RequiresApproval), + ]), + register: RefCell::new(Some(RegisterFailure::Failed(register_refusal()))), + exchanges: RefCell::new(vec![Peer::Unreachable; VERIFY_LISTEN_ATTEMPTS as usize]), + ..World::default() + }; + + assert_eq!(fresh_run(&world), Reconciled::PendingApproval); + assert_eq!(world.acts(), vec![Act::Registered]); + } + + #[test] + fn register_error_does_not_reclassify_a_broken_definition_after_verification() { + let world = World { + status: RefCell::new(vec![ + Ok(RegistrationStatus::NotRegistered), + Ok(RegistrationStatus::Enabled), + Ok(RegistrationStatus::NotFound), + ]), + definition: RefCell::new(vec![ + Ok(()), + Err(ServiceDefinitionError::Invalid( + "helper executable disappeared".to_string(), + )), + ]), + register: RefCell::new(Some(RegisterFailure::Failed(register_refusal()))), + exchanges: RefCell::new(vec![Peer::Unreachable; VERIFY_LISTEN_ATTEMPTS as usize]), + ..World::default() + }; + + assert_eq!(fresh_run(&world), Reconciled::ServiceDefinitionBroken); + assert_eq!(world.acts(), vec![Act::Registered]); + } + + #[test] + fn a_silent_fresh_register_uses_the_same_status_judgment() { + let world = World { + status: RefCell::new(vec![ + Ok(RegistrationStatus::NotRegistered), + Ok(RegistrationStatus::RequiresApproval), + ]), + register: RefCell::new(Some(RegisterFailure::Silent)), + ..World::default() + }; + + assert_eq!(fresh_run(&world), Reconciled::PendingApproval); + assert_eq!(world.acts(), vec![Act::Registered]); + } + + #[test] + fn first_time_grant_surfaces_approval_even_when_register_reports_an_error() { + let world = World { + status: RefCell::new(vec![ + Ok(RegistrationStatus::NotRegistered), + Ok(RegistrationStatus::RequiresApproval), + ]), + preference: RefCell::new(Ok(HelperPreference::Unset)), + register: RefCell::new(Some(RegisterFailure::Failed(register_refusal()))), + ..World::default() + }; + + assert_eq!( + decide(&Mutex::new(()), &world, HelperDecision::Granted), + Reconciled::PendingApproval + ); + assert_eq!(world.acts(), vec![Act::StoredGranted, Act::Registered]); + } + + #[test] + fn a_replacement_register_error_uses_the_same_status_judgment() { + let world = World { + status: RefCell::new(vec![ + Ok(RegistrationStatus::Enabled), + Ok(RegistrationStatus::RequiresApproval), + ]), + exchanges: RefCell::new(vec![Peer::Answered(status_reply( + OTHER_BUILD, + HelperStateName::Idle, + None, + ))]), + replace: RefCell::new(Some(ReplaceFailure::RegisterFailed(register_refusal()))), + ..World::default() + }; + + assert_eq!(fresh_run(&world), Reconciled::PendingApproval); + assert_eq!(world.acts(), vec![Act::Replaced]); + + let world = World { + status: RefCell::new(vec![ + Ok(RegistrationStatus::Enabled), + Ok(RegistrationStatus::Enabled), + ]), + exchanges: RefCell::new(vec![ + Peer::Answered(status_reply(OTHER_BUILD, HelperStateName::Idle, None)), + Peer::Answered(this_build_idle()), + ]), + replace: RefCell::new(Some(ReplaceFailure::RegisterSilent)), + ..World::default() + }; + assert_eq!(fresh_run(&world), Reconciled::AtThisBuild); + assert_eq!(world.acts(), vec![Act::Replaced]); + } + #[test] fn a_reply_that_is_not_a_status_is_a_refusal() { let world = World::answering(HelperReply::RequestNotUnderstood); @@ -1635,4 +2084,57 @@ mod tests { assert_eq!(acts, vec![Act::StoredGranted, Act::Registered]); }); } + + #[test] + fn retry_never_installs_for_an_undecided_or_disabled_user() { + for preference in [HelperPreference::Unset, HelperPreference::Disabled] { + let world = World { + preference: RefCell::new(Ok(preference)), + status: RefCell::new(vec![Ok(RegistrationStatus::NotRegistered)]), + ..World::default() + }; + assert_eq!(retry_with(&Mutex::new(()), &world), Reconciled::NoHelper); + assert!(world.acts().is_empty()); + assert_eq!(world.preference(), Ok(preference)); + } + } + + #[test] + fn retry_carries_out_a_stored_disable_without_rewriting_it() { + let world = World { + preference: RefCell::new(Ok(HelperPreference::Disabled)), + status: RefCell::new(vec![Ok(RegistrationStatus::RequiresApproval)]), + ..World::default() + }; + assert_eq!(retry_with(&Mutex::new(()), &world), Reconciled::Removed); + assert_eq!(world.acts(), vec![Act::Unregistered]); + assert_eq!(world.preference(), Ok(HelperPreference::Disabled)); + } + + #[test] + fn retry_waits_for_a_real_pass_without_rewriting_the_decision() { + let running = Mutex::new(()); + let held = running.lock().expect("a fresh lock"); + std::thread::scope(|scope| { + let retried = scope.spawn(|| { + let world = World { + status: RefCell::new(vec![ + Ok(RegistrationStatus::NotRegistered), + Ok(RegistrationStatus::Enabled), + ]), + exchanges: RefCell::new(vec![Peer::Answered(this_build_idle())]), + ..World::default() + }; + let report = retry_with(&running, &world); + (report, world.acts()) + }); + std::thread::sleep(Duration::from_millis(100)); + assert!(!retried.is_finished(), "retry did not wait for the slot"); + drop(held); + + let (report, acts) = retried.join().expect("retry"); + assert_eq!(report, Reconciled::AtThisBuild); + assert_eq!(acts, vec![Act::Registered]); + }); + } } diff --git a/apps/native/src-tauri/src/privileged_helper/service.rs b/apps/native/src-tauri/src/privileged_helper/service.rs index c0e728cce..327660f89 100644 --- a/apps/native/src-tauri/src/privileged_helper/service.rs +++ b/apps/native/src-tauri/src/privileged_helper/service.rs @@ -22,7 +22,8 @@ pub enum RegistrationStatus { /// consent the user revoked there. An approval-pending service has no /// process. RequiresApproval, - /// The bundle's service definition could not be found. + /// ServiceManagement has no record for this service, or could not find its + /// definition. A valid, never-registered bundled daemon can report this too. NotFound, } @@ -53,6 +54,75 @@ impl std::fmt::Display for RegistrationStatus { } } +/// A bundle inspection is separate from the framework's registration status. +/// In particular, `notFound` alone does not establish a broken installation. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ServiceDefinitionError { + Invalid(String), + Unavailable(String), +} + +/// Check the definition needed by our bundled daemon before treating a +/// framework `notFound` as a first registration. ServiceManagement still owns +/// registration/signature validation, and an authenticated reply owns readiness. +pub fn check_service_definition(bundle: &std::path::Path) -> Result<(), ServiceDefinitionError> { + use crate::privileged_helper::protocol::{HELPER_LABEL, HELPER_PLIST_NAME}; + use std::fs; + + let plist_path = bundle + .join("Contents/Library/LaunchDaemons") + .join(HELPER_PLIST_NAME); + let bytes = fs::read(&plist_path).map_err(|error| definition_io_error(&plist_path, error))?; + let value = plist::Value::from_reader(std::io::Cursor::new(bytes)).map_err(|error| { + ServiceDefinitionError::Invalid(format!("invalid helper plist: {error}")) + })?; + let dictionary = value.as_dictionary().ok_or_else(|| { + ServiceDefinitionError::Invalid("helper plist is not a dictionary".to_string()) + })?; + if dictionary.get("Label").and_then(plist::Value::as_string) != Some(HELPER_LABEL) { + return Err(ServiceDefinitionError::Invalid( + "helper plist label does not match".to_string(), + )); + } + const BUNDLE_PROGRAM: &str = "Contents/MacOS/nixmac-helper"; + if dictionary + .get("BundleProgram") + .and_then(plist::Value::as_string) + != Some(BUNDLE_PROGRAM) + || dictionary.contains_key("Program") + { + return Err(ServiceDefinitionError::Invalid( + "helper plist program does not match".to_string(), + )); + } + let program = bundle.join(BUNDLE_PROGRAM); + let metadata = fs::metadata(&program).map_err(|error| definition_io_error(&program, error))?; + if !metadata.is_file() { + return Err(ServiceDefinitionError::Invalid( + "bundled helper is not a regular file".to_string(), + )); + } + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + if metadata.permissions().mode() & 0o111 == 0 { + return Err(ServiceDefinitionError::Invalid( + "bundled helper is not executable".to_string(), + )); + } + } + Ok(()) +} + +fn definition_io_error(path: &std::path::Path, error: std::io::Error) -> ServiceDefinitionError { + let detail = format!("{}: {error}", path.display()); + if error.kind() == std::io::ErrorKind::NotFound { + ServiceDefinitionError::Invalid(detail) + } else { + ServiceDefinitionError::Unavailable(detail) + } +} + /// A failed ServiceManagement call, preserved structurally. /// /// The `NSError` domain and code are the identity of the failure; the localized @@ -95,8 +165,9 @@ pub enum ReplaceFailure { /// helper is registered now. The reason is the caller's own: this module /// supplies the point where such a decision is possible and takes none. RegisterDeclined(E), - /// The old process is gone and the replacement was refused: no helper is - /// registered now. + /// The old process is gone and the replacement reported a refusal. macOS + /// can still move the service to approval-required or enabled, so callers + /// must re-read status before describing the resulting state. RegisterFailed(ServiceCallError), /// The old process is gone and the replacement never reported. Whether it /// took is unknown — only a fresh observation can say. @@ -125,7 +196,8 @@ pub enum RegisterFailure { /// Refused before anything was dispatched: the main thread is the one that /// has to make the call, so awaiting it there would starve the queue. CalledOnMainThread, - /// The platform refused the registration; nothing is registered. + /// The platform reported a registration refusal. macOS can still move the + /// service to approval-required or enabled, so callers re-read status. Failed(ServiceCallError), /// The call never reported inside the window. Whether it took is unknown — /// only a fresh observation can say. @@ -566,6 +638,101 @@ mod tests { use super::*; use std::cell::Cell; + fn valid_bundle() -> tempfile::TempDir { + use std::fs; + let directory = tempfile::tempdir().expect("temporary app bundle"); + let bundle = directory.path(); + let plist = + bundle.join("Contents/Library/LaunchDaemons/com.darkmatter.nixmac.helper.plist"); + fs::create_dir_all(plist.parent().unwrap()).unwrap(); + fs::write( + &plist, + include_str!("../../resources/launchd/com.darkmatter.nixmac.helper.plist"), + ) + .unwrap(); + let program = bundle.join("Contents/MacOS/nixmac-helper"); + fs::create_dir_all(program.parent().unwrap()).unwrap(); + fs::write(&program, b"test executable").unwrap(); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + fs::set_permissions(program, fs::Permissions::from_mode(0o755)).unwrap(); + } + directory + } + + #[test] + fn a_bundled_definition_is_valid_without_a_framework_registration_record() { + let bundle = valid_bundle(); + assert_eq!(check_service_definition(bundle.path()), Ok(())); + } + + #[test] + fn a_missing_plist_or_helper_executable_is_a_broken_definition() { + for relative in [ + "Contents/Library/LaunchDaemons/com.darkmatter.nixmac.helper.plist", + "Contents/MacOS/nixmac-helper", + ] { + let bundle = valid_bundle(); + std::fs::remove_file(bundle.path().join(relative)).unwrap(); + assert!(matches!( + check_service_definition(bundle.path()), + Err(ServiceDefinitionError::Invalid(_)) + )); + } + } + + #[test] + fn malformed_or_mismatched_launchd_definitions_are_rejected() { + let template = include_str!("../../resources/launchd/com.darkmatter.nixmac.helper.plist"); + for contents in [ + "not a plist".to_string(), + template.replace("com.darkmatter.nixmac.helper", "com.example.other-helper"), + template.replace("Contents/MacOS/nixmac-helper", "../outside-helper"), + template.replace("BundleProgram", "Program"), + ] { + let bundle = valid_bundle(); + std::fs::write( + bundle + .path() + .join("Contents/Library/LaunchDaemons/com.darkmatter.nixmac.helper.plist"), + contents, + ) + .unwrap(); + assert!(matches!( + check_service_definition(bundle.path()), + Err(ServiceDefinitionError::Invalid(_)) + )); + } + } + + #[cfg(unix)] + #[test] + fn a_non_executable_helper_is_a_broken_definition() { + use std::os::unix::fs::PermissionsExt; + let bundle = valid_bundle(); + std::fs::set_permissions( + bundle.path().join("Contents/MacOS/nixmac-helper"), + std::fs::Permissions::from_mode(0o644), + ) + .unwrap(); + assert!(matches!( + check_service_definition(bundle.path()), + Err(ServiceDefinitionError::Invalid(_)) + )); + } + + #[test] + fn a_definition_read_error_is_not_mislabeled_as_a_missing_file() { + assert!(matches!( + definition_io_error( + std::path::Path::new("helper.plist"), + std::io::ErrorKind::PermissionDenied.into() + ), + ServiceDefinitionError::Unavailable(_) + )); + } + #[test] fn the_four_smappservice_statuses_map_by_raw_value() { // The raw values are Apple's enum order; a wrong mapping would send the diff --git a/apps/native/src-tauri/src/rebuild/darwin.rs b/apps/native/src-tauri/src/rebuild/darwin.rs index 47e1df166..877ec1ea7 100644 --- a/apps/native/src-tauri/src/rebuild/darwin.rs +++ b/apps/native/src-tauri/src/rebuild/darwin.rs @@ -368,11 +368,45 @@ fn app_management_error_payload( /// /// Emits the terminal `darwin:apply:end` event and records the outcome in /// the rebuild-status cell (which emits `rebuild_status_changed`). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum ApplyEndAttention { + Silent, + BuildBlocked, + BuildFailed, +} + +fn apply_end_attention(payload: &serde_json::Value) -> ApplyEndAttention { + if payload.get("ok").and_then(|value| value.as_bool()) != Some(false) { + return ApplyEndAttention::Silent; + } + match payload.get("error_type").and_then(|value| value.as_str()) { + // The user cancelled the macOS prompt themselves, so there is nothing + // new to tell them while nixmac is hidden. + Some("user_cancelled") => ApplyEndAttention::Silent, + // A blocker the user can resolve (another activation running, files in + // /etc to rename, the App Management permission, administrator + // credentials), not a failed build. `authorization_denied` stays + // visible: its classifier also matches broad text such as "not + // permitted", so silence could hide an activation the user never + // declined. + Some("activation_refused" | "etc_clobber" | "app_management" | "authorization_denied") => { + ApplyEndAttention::BuildBlocked + } + _ => ApplyEndAttention::BuildFailed, + } +} + fn emit_apply_end(app: &AppHandle, payload: serde_json::Value) { + let attention = apply_end_attention(&payload); crate::state::rebuild_status::record_end(app, &payload); // fire-and-forget: emit returns Err only when no listeners are registered // (window may be hidden/destroyed). Missing this event is non-fatal. let _ = app.emit("darwin:apply:end", payload); + match attention { + ApplyEndAttention::Silent => {} + ApplyEndAttention::BuildBlocked => crate::attention::build_blocked(app), + ApplyEndAttention::BuildFailed => crate::attention::build_failed(app), + } } pub fn apply_stream( @@ -721,6 +755,9 @@ pub fn activate_store_path_stream( serde_json::json!({"chunk": "Activating previous nix store...\n"}), ); + if crate::main_window::active(&app_handle).is_popover() { + crate::state::rebuild_status::expect_owned_store_path_change(store_path.clone()); + } match activate_store_path(&app_handle, &store_path) { Ok(result) => { for line in result.stdout.lines() { @@ -736,7 +773,11 @@ pub fn activate_store_path_stream( info!("[darwin] store path activation succeeded"); emit_apply_end( &app_handle, - serde_json::json!({"ok": true, "code": result.code}), + serde_json::json!({ + "ok": true, + "code": result.code, + "store_path": store_path, + }), ); } else { let (error_type, error) = classify_activate_error(&result); @@ -1216,6 +1257,11 @@ fn run_darwin_rebuild( })); }; + if crate::main_window::active(app).is_popover() { + crate::state::rebuild_status::expect_owned_store_path_change( + system_store_path.to_string_lossy().to_string(), + ); + } let activate_result = run_activate_step(app, system_store_path).map_err(|e| { serde_json::json!({ "ok": false, @@ -1359,6 +1405,7 @@ fn run_darwin_rebuild( "ok": true, "code": activate_result.code, "log_file": log_path.to_string_lossy(), + "store_path": system_store_path.to_string_lossy(), }); if let Some(et) = error_type && let Some(obj) = success_payload.as_object_mut() @@ -1374,7 +1421,9 @@ fn run_darwin_rebuild( #[cfg(test)] mod activation_safety_tests { use super::{ - ActivateResult, activation_failure_left_system_untouched, classify_activate_error, + ActivateResult, ApplyEndAttention, activation_failure_left_system_untouched, + app_management_error_payload, apply_end_attention, classify_activate_error, + etc_clobber_error_payload, handle_activation_error, }; fn failed_activation(stdout: &str, stderr: &str) -> ActivateResult { @@ -1455,4 +1504,98 @@ mod activation_safety_tests { )); assert!(!activation_failure_left_system_untouched("generic_error")); } + + fn attention_for(error_type: &str) -> ApplyEndAttention { + apply_end_attention(&serde_json::json!({ + "ok": false, + "error_type": error_type, + })) + } + + #[test] + fn apply_end_attention_is_silent_for_success_and_user_cancellation() { + assert_eq!( + apply_end_attention(&serde_json::json!({ "ok": true })), + ApplyEndAttention::Silent + ); + assert_eq!( + attention_for("user_cancelled"), + ApplyEndAttention::Silent, + "user_cancelled is the user's own answer to a macOS prompt" + ); + } + + #[test] + fn apply_end_attention_reports_user_actionable_blockers_as_blocked() { + for error_type in [ + "activation_refused", + "etc_clobber", + "app_management", + "authorization_denied", + ] { + assert_eq!( + attention_for(error_type), + ApplyEndAttention::BuildBlocked, + "{error_type} is a blocker the user can resolve, not a failed build" + ); + } + } + + #[test] + fn apply_end_attention_reports_other_failures_as_failed() { + for error_type in ["generic_error", "build_error", "an_unknown_type"] { + assert_eq!( + attention_for(error_type), + ApplyEndAttention::BuildFailed, + "{error_type} must not be hidden or downgraded" + ); + } + assert_eq!( + apply_end_attention(&serde_json::json!({ "ok": false })), + ApplyEndAttention::BuildFailed + ); + } + + #[test] + fn apply_end_attention_matches_the_payloads_rebuild_actually_emits() { + let log_path = std::path::Path::new("/tmp/nixmac-apply.log"); + let etc_clobber = etc_clobber_error_payload( + crate::shared_types::EtcClobberCheckResult { + ok: false, + checked: 1, + conflicts: Vec::new(), + warnings: Vec::new(), + }, + log_path, + ); + let app_management_preflight = app_management_error_payload( + crate::shared_types::AppManagementCheckResult { + ok: false, + checked: 1, + targets: Vec::new(), + failures: Vec::new(), + }, + log_path, + ); + let app_management_activation = handle_activation_error( + &failed_activation("", "Permission denied when trying to update apps"), + log_path, + ); + let authorization_denied = + handle_activation_error(&failed_activation("", "authorization failed"), log_path); + let user_cancelled = + handle_activation_error(&failed_activation("", "User canceled. (-128)"), log_path); + let generic = handle_activation_error(&failed_activation("", "boom"), log_path); + + for (payload, expected) in [ + (&etc_clobber, ApplyEndAttention::BuildBlocked), + (&app_management_preflight, ApplyEndAttention::BuildBlocked), + (&app_management_activation, ApplyEndAttention::BuildBlocked), + (&authorization_denied, ApplyEndAttention::BuildBlocked), + (&user_cancelled, ApplyEndAttention::Silent), + (&generic, ApplyEndAttention::BuildFailed), + ] { + assert_eq!(apply_end_attention(payload), expected, "{payload}"); + } + } } diff --git a/apps/native/src-tauri/src/rebuild/finalize_apply.rs b/apps/native/src-tauri/src/rebuild/finalize_apply.rs index cbb5518a0..fa4b092f0 100644 --- a/apps/native/src-tauri/src/rebuild/finalize_apply.rs +++ b/apps/native/src-tauri/src/rebuild/finalize_apply.rs @@ -50,6 +50,7 @@ pub async fn finalize_apply(app: &AppHandle) -> Result<()> { log::warn!("Failed to record onboarding build timestamp: {error:#}"); } evolve_state::set_session(app, current_evolve, &final_status.changes)?; + crate::attention::build_succeeded(app); Ok(()) } @@ -71,5 +72,6 @@ pub async fn finalize_rollback( // The rollback restored an earlier tree: refresh the change-map cell so // the mirrored map matches it (emits `change_map_changed`). crate::summarize::refresh_change_map(app); + crate::attention::restore_finished(app); Ok(()) } diff --git a/apps/native/src-tauri/src/shared_types/system.rs b/apps/native/src-tauri/src/shared_types/system.rs index 051a2747e..2419c4274 100644 --- a/apps/native/src-tauri/src/shared_types/system.rs +++ b/apps/native/src-tauri/src/shared_types/system.rs @@ -57,6 +57,24 @@ pub enum PermissionStatus { Unknown, } +/// Product-facing state of the unattended sync helper. +/// +/// The helper is represented in the permissions list, but its lifecycle has +/// more states than a normal permission grant. Keeping those states typed lets +/// every UI surface use the same copy and actions without parsing diagnostic +/// text or overloading [`PermissionStatus`]. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Type, Copy)] +#[serde(rename_all = "camelCase")] +pub enum HelperPermissionPhase { + Ready, + ApprovalRequired, + Reconciling, + WaitingForActivation, + NeedsUserAction, + Failed, + Disabled, +} + /// Individual permission state. #[derive(Debug, Clone, Serialize, Deserialize, Type)] #[serde(rename_all = "camelCase")] @@ -83,6 +101,9 @@ pub struct Permission { /// Manual instructions for permissions that cannot be requested programmatically. #[serde(skip_serializing_if = "Option::is_none")] pub instructions: Option, + /// Helper-specific lifecycle state. Absent for normal permission rows. + #[serde(skip_serializing_if = "Option::is_none")] + pub helper_phase: Option, } /// All permissions state. diff --git a/apps/native/src-tauri/src/state/drift_notifications.rs b/apps/native/src-tauri/src/state/drift_notifications.rs index a1769158b..8b11a1c77 100644 --- a/apps/native/src-tauri/src/state/drift_notifications.rs +++ b/apps/native/src-tauri/src/state/drift_notifications.rs @@ -1,10 +1,14 @@ //! Native notifications for configuration drift detected by the watcher. use std::sync::Mutex; +use std::sync::atomic::{AtomicU64, Ordering}; -use crate::shared_types::GitStatus; +use tauri::{AppHandle, Runtime}; + +use crate::shared_types::{EvolveSession, GitStatus}; static LAST_DRIFT_NOTIFICATION_ID: Mutex> = Mutex::new(None); +static DRIFT_GENERATION: AtomicU64 = AtomicU64::new(0); #[derive(Debug, Clone, PartialEq, Eq)] struct DriftNotification { @@ -15,31 +19,93 @@ struct DriftNotification { pub fn maybe_notify(git_status: Option<&GitStatus>, external_build_detected: bool) { let notification = notification_for_event(git_status, external_build_detected); - let mut last_notification_id = match LAST_DRIFT_NOTIFICATION_ID.lock() { - Ok(guard) => guard, - Err(poisoned) => poisoned.into_inner(), - }; let Some(notification) = notification else { - *last_notification_id = None; + set_last_notification_id(None); + DRIFT_GENERATION.fetch_add(1, Ordering::SeqCst); + if let Some(app) = crate::APP_HANDLE.get() + && crate::main_window::active(app).is_popover() + { + crate::attention::clear_drift(app); + } return; }; - if last_notification_id.as_deref() == Some(notification.id.as_str()) { + if let Some(app) = crate::APP_HANDLE.get() + && crate::main_window::active(app).is_popover() + { + if notification.id != "external-build" + && (crate::evolve::session_control::is_evolve_active() + || session_owns_current_changes(app, &crate::state::evolve_state::get_session(app))) + { + return; + } + // The attention backend owns popover deduplication because it knows + // whether the window was unattended when delivery was attempted. + // A focused-window observation must not consume a future notice. + let generation = if notification.id == "external-build" { + DRIFT_GENERATION.fetch_add(1, Ordering::SeqCst) + 1 + } else { + DRIFT_GENERATION.load(Ordering::SeqCst) + }; + let attention_id = attention_drift_id(¬ification.id, generation); + crate::attention::drift(app, &attention_id, notification.title, ¬ification.body); return; } - // Don't let the one-shot external-build notification disrupt config-drift deduping. - if notification.id != "external-build" { - *last_notification_id = Some(notification.id.clone()); + if !claim_notification_id(¬ification.id, notification.id != "external-build") { + return; } - drop(last_notification_id); - if let Err(error) = send_native_notification(notification.title, ¬ification.body) { log::warn!("Failed to send drift notification: {error}"); } } +fn set_last_notification_id(id: Option) { + match LAST_DRIFT_NOTIFICATION_ID.lock() { + Ok(mut guard) => *guard = id, + Err(poisoned) => *poisoned.into_inner() = id, + } +} + +fn claim_notification(last: &mut Option, id: &str, remember: bool) -> bool { + if last.as_deref() == Some(id) { + return false; + } + // Don't let the one-shot external-build notification disrupt config-drift deduping. + if remember { + *last = Some(id.to_string()); + } + true +} + +fn claim_notification_id(id: &str, remember: bool) -> bool { + let mut last = match LAST_DRIFT_NOTIFICATION_ID.lock() { + Ok(guard) => guard, + Err(poisoned) => poisoned.into_inner(), + }; + claim_notification(&mut last, id, remember) +} + +fn attention_drift_id(notification_id: &str, generation: u64) -> String { + format!("{notification_id}:{generation}") +} + +fn session_owns_current_changes(app: &AppHandle, session: &EvolveSession) -> bool { + let (Some(_), Some(snapshot_id)) = (session.evolution_id, session.current_changeset_id) else { + return false; + }; + // A pending session can outlive the agent run. Only its exact recorded + // changes are covered by completion attention; later manual edits need + // their own drift notice. Use the same base ref as the stored snapshot, + // which may include a backup of manual changes that preceded the run. + let base_ref = crate::summarize::active_summary_base_ref(app); + crate::summarize::found_since(app, &base_ref) + .ok() + .flatten() + .is_some_and(|found| found.snapshot_id == Some(snapshot_id)) +} + fn notification_for_event( git_status: Option<&GitStatus>, external_build_detected: bool, @@ -80,6 +146,11 @@ fn send_native_notification(title: &str, body: &str) -> Result<(), String> { let app_handle = crate::APP_HANDLE .get() .ok_or("App handle not initialized")?; + if !crate::attention::plugin_notification_backend_allowed(app_handle) { + return Err( + "plugin-backed notifications are disabled in menu-bar popover mode".to_string(), + ); + } app_handle .notification() @@ -150,4 +221,91 @@ mod tests { }) ); } + + #[tokio::test] + async fn pending_evolution_only_suppresses_its_recorded_changes() { + use crate::observable::Observable; + use crate::shared_types::GlobalPreferences; + use tauri::Manager; + + let temp = tempfile::tempdir().unwrap(); + let config_dir = temp.path().join("config"); + std::fs::create_dir(&config_dir).unwrap(); + let repo = git2::Repository::init(&config_dir).unwrap(); + let config_file = config_dir.join("flake.nix"); + std::fs::write(&config_file, "initial\n").unwrap(); + let mut index = repo.index().unwrap(); + index.add_path(std::path::Path::new("flake.nix")).unwrap(); + index.write().unwrap(); + let tree_id = index.write_tree().unwrap(); + let tree = repo.find_tree(tree_id).unwrap(); + let signature = git2::Signature::now("nixmac", "nixmac@local").unwrap(); + repo.commit(Some("HEAD"), &signature, &signature, "initial", &tree, &[]) + .unwrap(); + + // The agent started from pre-existing manual changes, so its stored + // snapshot is relative to the backup rather than HEAD. + std::fs::write(&config_file, "manual baseline\n").unwrap(); + let dir = config_dir.to_string_lossy().to_string(); + let backup = crate::git::create_evolution_backup(&dir, None, 0) + .unwrap() + .unwrap(); + std::fs::write(&config_file, "agent result\n").unwrap(); + let hashes = crate::git::query::changes_since_ref(&dir, &backup) + .unwrap() + .into_iter() + .map(|diff| crate::git::file_diff_to_change(diff, 0, false).hash) + .collect::>(); + let pool = crate::db::init_pool_at_path(&temp.path().join("nixmac.db")) + .await + .unwrap(); + let snapshot_id = crate::db::snapshots::upsert( + &pool, + &crate::db::keys::snapshot_key(&hashes), + None, + None, + 0, + ) + .unwrap(); + let session = EvolveSession { + evolution_id: Some(1), + current_changeset_id: Some(snapshot_id), + rollback_branch: Some(backup), + ..Default::default() + }; + let app = tauri::test::mock_builder() + .build(tauri::test::mock_context(tauri::test::noop_assets())) + .unwrap(); + app.manage(pool); + app.manage(Observable::new(GlobalPreferences { + config_dir: Some(dir), + ..Default::default() + })); + app.manage(Observable::new(session.clone())); + + assert!(session_owns_current_changes(app.handle(), &session)); + std::fs::write(&config_file, "manual edit after generation\n").unwrap(); + assert!(!session_owns_current_changes(app.handle(), &session)); + assert!(!session_owns_current_changes( + app.handle(), + &EvolveSession::default() + )); + } + + #[test] + fn control_mode_keeps_existing_dedupe_behavior() { + let mut last = None; + assert!(claim_notification(&mut last, "config-drift:abc123", true)); + assert!(!claim_notification(&mut last, "config-drift:abc123", true)); + assert!(claim_notification(&mut last, "external-build", false)); + assert_eq!(last.as_deref(), Some("config-drift:abc123")); + } + + #[test] + fn a_clean_transition_allows_the_same_head_to_notify_again() { + assert_ne!( + attention_drift_id("config-drift:abc123", 1), + attention_drift_id("config-drift:abc123", 2) + ); + } } diff --git a/apps/native/src-tauri/src/state/rebuild_status.rs b/apps/native/src-tauri/src/state/rebuild_status.rs index d66692e25..5c2b3b1dc 100644 --- a/apps/native/src-tauri/src/state/rebuild_status.rs +++ b/apps/native/src-tauri/src/state/rebuild_status.rs @@ -5,12 +5,21 @@ //! line-by-line output intentionally stays on the streams. Not persisted — //! a fresh process has no rebuild in flight. +use std::sync::Mutex; + use tauri::{AppHandle, Manager, Runtime}; use crate::observable::Observable; use crate::shared_types::RebuildStatus; pub const REBUILD_STATUS_CHANGED_EVENT: &str = "rebuild_status_changed"; +static OWNED_STORE_PATH_CHANGE: Mutex> = Mutex::new(None); + +#[derive(Debug, PartialEq, Eq)] +struct OwnedStorePathChange { + path: String, + completed: bool, +} pub fn load_observable(app: &AppHandle) -> Observable { Observable::new(RebuildStatus::default()).emit_to(app, REBUILD_STATUS_CHANGED_EVENT) @@ -23,12 +32,15 @@ pub fn get(app: &AppHandle) -> RebuildStatus { /// Clear the last-known rebuild status. pub fn reset(app: &AppHandle) { + clear_owned_store_path_change(); let observable = app.state::>(); *observable.write_sync() = RebuildStatus::default(); } /// Record the start of a rebuild stream; clears the previous run's outcome. pub fn record_start(app: &AppHandle) { + crate::attention::clear_work(app); + clear_owned_store_path_change(); let observable = app.state::>(); *observable.write_sync() = RebuildStatus { is_running: true, @@ -38,6 +50,12 @@ pub fn record_start(app: &AppHandle) { /// Record the end of a rebuild stream from the `darwin:apply:end` payload. pub fn record_end(app: &AppHandle, payload: &serde_json::Value) { + { + let mut expected = OWNED_STORE_PATH_CHANGE + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + finish_owned_store_path_change(&mut expected, payload); + } let observable = app.state::>(); *observable.write_sync() = RebuildStatus { is_running: false, @@ -58,11 +76,94 @@ pub fn record_end(app: &AppHandle, payload: &serde_json::Value) { }; } +fn finish_owned_store_path_change( + expected: &mut Option, + payload: &serde_json::Value, +) { + if payload.get("ok").and_then(|value| value.as_bool()) == Some(true) { + if let Some(expected) = expected + && payload.get("store_path").and_then(|value| value.as_str()) + == Some(expected.path.as_str()) + { + expected.completed = true; + } + } else { + *expected = None; + } +} + +pub fn expect_owned_store_path_change(store_path: String) { + let expected = Some(OwnedStorePathChange { + path: store_path, + completed: false, + }); + match OWNED_STORE_PATH_CHANGE.lock() { + Ok(mut guard) => *guard = expected, + Err(poisoned) => *poisoned.into_inner() = expected, + } +} + +fn consume_owned_store_path( + expected: &mut Option, + live: &Option, +) -> bool { + let (Some(pending), Some(live)) = (expected.as_ref(), live.as_ref()) else { + return false; + }; + let matches = pending.path == *live; + // Before completion the system can still report the previous path. After + // success, a different observed path means another build has superseded it. + if matches || pending.completed { + *expected = None; + } + matches +} + +fn sample_owned_store_path_change( + tracker: &Mutex>, + read_live: impl FnOnce() -> Option, +) -> (Option, bool) { + // Serialize sampling with completion. Reading before taking this lock + // could turn a pre-completion old-path sample into a completed mismatch. + let mut expected = match tracker.lock() { + Ok(guard) => guard, + Err(poisoned) => poisoned.into_inner(), + }; + let live = read_live(); + let owned = consume_owned_store_path(&mut expected, &live); + (live, owned) +} + +pub fn read_owned_store_path_change() -> (Option, bool) { + sample_owned_store_path_change( + &OWNED_STORE_PATH_CHANGE, + super::build_state::read_current_store_path, + ) +} + +pub fn clear_owned_store_path_change() { + match OWNED_STORE_PATH_CHANGE.lock() { + Ok(mut guard) => *guard = None, + Err(poisoned) => *poisoned.into_inner() = None, + } +} + #[cfg(test)] mod tests { use super::*; use tauri::Manager; + fn pending(path: &str) -> Option { + Some(OwnedStorePathChange { + path: path.to_string(), + completed: false, + }) + } + + fn success(path: &str) -> serde_json::Value { + serde_json::json!({ "ok": true, "store_path": path }) + } + fn mock_app() -> tauri::App { let app = tauri::test::mock_builder() .build(tauri::test::mock_context(tauri::test::noop_assets())) @@ -92,4 +193,143 @@ mod tests { assert_eq!(get(handle), RebuildStatus::default()); } + + #[test] + fn only_the_exact_owned_store_path_transition_is_suppressed() { + let mut expected = pending("/nix/store/owned"); + assert!(!consume_owned_store_path( + &mut expected, + &Some("/nix/store/external".to_string()) + )); + assert_eq!(expected, pending("/nix/store/owned")); + + expected = pending("/nix/store/owned"); + assert!(consume_owned_store_path( + &mut expected, + &Some("/nix/store/owned".to_string()) + )); + assert_eq!(expected, None); + } + + #[test] + fn owned_transition_survives_until_the_delayed_matching_watcher_poll() { + let mut expected = pending("/nix/store/new-system"); + + // Polls before activation completes must not consume the expectation. + assert!(!consume_owned_store_path(&mut expected, &None)); + assert_eq!(expected, pending("/nix/store/new-system")); + assert!(!consume_owned_store_path( + &mut expected, + &Some("/nix/store/old-system".to_string()) + )); + assert_eq!(expected, pending("/nix/store/new-system")); + + finish_owned_store_path_change(&mut expected, &success("/nix/store/new-system")); + + // The first poll that observes the activated path consumes it once. + assert!(consume_owned_store_path( + &mut expected, + &Some("/nix/store/new-system".to_string()) + )); + assert!(!consume_owned_store_path( + &mut expected, + &Some("/nix/store/new-system".to_string()) + )); + } + + #[test] + fn successful_end_keeps_expectation_but_failure_releases_it() { + let mut expected = pending("/nix/store/owned"); + finish_owned_store_path_change(&mut expected, &success("/nix/store/owned")); + assert_eq!( + expected, + Some(OwnedStorePathChange { + path: "/nix/store/owned".to_string(), + completed: true, + }) + ); + for payload in [serde_json::json!({ "ok": false }), serde_json::json!({})] { + let mut expected = pending("/nix/store/owned"); + finish_owned_store_path_change(&mut expected, &payload); + assert_eq!(expected, None); + } + } + + #[test] + fn intervening_external_build_expires_the_completed_owned_path() { + let mut expected = pending("/nix/store/a"); + finish_owned_store_path_change(&mut expected, &success("/nix/store/a")); + + // A completed while unfocused, but the next watcher poll first sees B. + assert!(!consume_owned_store_path( + &mut expected, + &Some("/nix/store/b".to_string()) + )); + assert_eq!(expected, None); + // A later external rollback to A must not inherit the old ownership, + // including after the user acknowledges B as the active build. + assert!(!consume_owned_store_path( + &mut expected, + &Some("/nix/store/a".to_string()) + )); + } + + #[test] + fn an_unavailable_post_completion_read_does_not_prove_an_intervening_build() { + let mut expected = pending("/nix/store/a"); + finish_owned_store_path_change(&mut expected, &success("/nix/store/a")); + assert!(!consume_owned_store_path(&mut expected, &None)); + assert!(consume_owned_store_path( + &mut expected, + &Some("/nix/store/a".to_string()) + )); + } + + #[test] + fn completion_does_not_rearm_an_owned_path_already_observed_during_activation() { + let mut expected = pending("/nix/store/a"); + assert!(consume_owned_store_path( + &mut expected, + &Some("/nix/store/a".to_string()) + )); + finish_owned_store_path_change(&mut expected, &success("/nix/store/a")); + assert_eq!(expected, None); + assert!(!consume_owned_store_path( + &mut expected, + &Some("/nix/store/a".to_string()) + )); + } + + #[test] + fn another_activations_success_cannot_complete_the_pending_owner() { + for payload in [success("/nix/store/a"), serde_json::json!({ "ok": true })] { + let mut expected = pending("/nix/store/b"); + finish_owned_store_path_change(&mut expected, &payload); + assert!(!consume_owned_store_path( + &mut expected, + &Some("/nix/store/a".to_string()) + )); + assert_eq!(expected, pending("/nix/store/b")); + } + } + + #[test] + fn sampling_cannot_cross_activation_completion() { + let tracker = Mutex::new(pending("/nix/store/a")); + let (live, owned) = sample_owned_store_path_change(&tracker, || { + // record_end needs this same lock to mark completion. It cannot + // settle between an old-path read and classification of that read. + assert!(matches!( + tracker.try_lock(), + Err(std::sync::TryLockError::WouldBlock) + )); + Some("/nix/store/old".to_string()) + }); + assert_eq!(live.as_deref(), Some("/nix/store/old")); + assert!(!owned); + assert_eq!(*tracker.lock().unwrap(), pending("/nix/store/a")); + + finish_owned_store_path_change(&mut tracker.lock().unwrap(), &success("/nix/store/a")); + assert!(sample_owned_store_path_change(&tracker, || Some("/nix/store/a".to_string())).1); + } } diff --git a/apps/native/src-tauri/src/state/watcher.rs b/apps/native/src-tauri/src/state/watcher.rs index 9825be5dd..bec22b9a5 100644 --- a/apps/native/src-tauri/src/state/watcher.rs +++ b/apps/native/src-tauri/src/state/watcher.rs @@ -117,7 +117,15 @@ fn check_for_external_build( app_handle: &AppHandle, last_known_store_path: &mut Option, ) -> bool { - let live_store_path = build_state::read_current_store_path(); + // Consume an owned activation even when it rebuilt the already-active + // path, or finalization updated the cached path before a watcher restart. + // Leaving that expectation behind could hide a later external restore. + let (live_store_path, owned_by_nixmac) = if crate::main_window::active(app_handle).is_popover() + { + crate::state::rebuild_status::read_owned_store_path_change() + } else { + (build_state::read_current_store_path(), false) + }; if live_store_path == *last_known_store_path { return false; } @@ -131,7 +139,7 @@ fn check_for_external_build( let nixmac_built = build_state::get(app_handle) .ok() .and_then(|state| state.nixmac_built_store_path); - live_store_path.is_some() && live_store_path != nixmac_built + live_store_path.is_some() && live_store_path != nixmac_built && !owned_by_nixmac } fn check_git_status( diff --git a/apps/native/src-tauri/src/system/helper_permission.rs b/apps/native/src-tauri/src/system/helper_permission.rs index 969a33733..07c5c04df 100644 --- a/apps/native/src-tauri/src/system/helper_permission.rs +++ b/apps/native/src-tauri/src/system/helper_permission.rs @@ -2,10 +2,10 @@ //! //! One reconciliation function decides everything about the installed helper //! (`privileged_helper::reconcile`). This module is the only place the GUI calls -//! it from, and it holds nothing but the wiring: +//! it from, and it owns the wiring plus the one shared presentation window: //! -//! * [`observe`] for startup and status refreshes, [`grant`] and [`disable`] -//! for the two explicit user actions; +//! * [`observe`] for startup and status refreshes, plus [`grant`], [`retry`], +//! and [`disable`] for explicit user actions; //! * [`describe`] and [`row`], which turn one run's report into the sentence //! and the permission state the UI shows. //! @@ -15,17 +15,23 @@ //! approval a report describes, a run no click is waiting on opens nothing. use std::sync::atomic::{AtomicBool, Ordering}; -use std::time::Duration; +use std::sync::{Mutex, PoisonError}; +use std::time::{Duration, Instant}; use tauri::{AppHandle, Runtime}; use crate::privileged_helper::reconcile::{self, LiveEnvironment, Reconciled}; use crate::privileged_helper::service::{self, RegistrationStatus}; -use crate::shared_types::PermissionStatus; +use crate::shared_types::{HelperPermissionPhase, PermissionStatus}; use crate::state::permissions_state; use crate::system::permissions; -const APPROVE_IN_LOGIN_ITEMS: &str = "Approve nixmac in System Settings → General → Login Items & Extensions to finish enabling the unattended sync helper."; +const APPROVE_IN_LOGIN_ITEMS: &str = "Allow nixmac in System Settings → General → Login Items & Extensions to finish enabling unattended sync."; +const FINISHING_SETUP: &str = "nixmac is finishing unattended sync setup."; +const HELPER_UPDATE_FAILED: &str = "nixmac couldn’t update the unattended sync helper. Try again. If this keeps happening, report the problem."; +const ACTION_FAILED: &str = "nixmac couldn’t finish changing the unattended sync helper. Try again. If this keeps happening, report the problem."; +const SERVICE_DEFINITION_BROKEN: &str = + "This copy of nixmac can’t install its unattended sync helper. Reinstall or update nixmac."; /// Reconciles the installed helper with this build, and reports. /// @@ -41,7 +47,19 @@ const APPROVE_IN_LOGIN_ITEMS: &str = "Approve nixmac in System Settings → Gene /// so a main-thread run that only has to read a status or remove a helper is not /// caught by that refusal. pub fn observe(app: &AppHandle) -> Reconciled { - reconcile::reconcile(&LiveEnvironment::new(app)) + observe_with(&PRESENTATION, || { + reconcile::reconcile(&LiveEnvironment::new(app)) + }) +} + +fn observe_with( + presentation: &FailurePresentation, + observe: impl FnOnce() -> Reconciled, +) -> Reconciled { + let generation = presentation.generation(); + let report = observe(); + presentation.note_settled_report(&report, generation); + report } /// The explicit Grant action: record the decision, reconcile under it, and open @@ -75,6 +93,7 @@ pub fn observe(app: &AppHandle) -> Reconciled { /// pass may be awaiting a main-queue dispatch — a deadlock, where an /// observation merely misreports. Every current caller is an async handler. pub fn grant(app: &AppHandle) -> Reconciled { + let generation = PRESENTATION.begin_user_attempt(); // Read before the run, because the run is what changes it. let awaiting_approval_already = matches!( service::registration_status(), @@ -84,6 +103,7 @@ pub fn grant(app: &AppHandle) -> Reconciled { service::open_login_items_settings(); } let report = reconcile::grant(&LiveEnvironment::new(app)); + PRESENTATION.note_settled_report(&report, generation); if !awaiting_approval_already && matches!(report, Reconciled::PendingApproval) { service::open_login_items_settings(); } @@ -91,23 +111,99 @@ pub fn grant(app: &AppHandle) -> Reconciled { report } +/// Retry the stored decision without changing it or opening System Settings. +/// +/// This is deliberately separate from [`grant`]: a failed, already-enabled +/// helper needs another reconciliation attempt, not another opt-in action. +pub fn retry(app: &AppHandle) -> Reconciled { + let generation = PRESENTATION.begin_user_attempt(); + let report = reconcile::retry(&LiveEnvironment::new(app)); + PRESENTATION.note_settled_report(&report, generation); + if matches!(report, Reconciled::Stopped(_)) { + // This was an explicit, bounded retry whose progress the button showed. + // If that pass still stopped, keep recovery available rather than + // leaving a spinner up until the slow background cadence wakes again. + PRESENTATION.show_failure(generation); + } + start_converging(app); + report +} + /// The explicit Disable action: record the decision, then reconcile under it — /// unregister the helper, deferring while an activation runs, done. Never on /// the main thread, for [`grant`]'s reason. pub fn disable(app: &AppHandle) -> Reconciled { + let generation = PRESENTATION.begin_user_attempt(); let report = reconcile::disable(&LiveEnvironment::new(app)); + PRESENTATION.note_settled_report(&report, generation); start_converging(app); report } /// The permission row for one report: only a helper of this build, ready, is /// granted, and every report says what it found. -pub fn row(report: &Reconciled) -> (PermissionStatus, String) { +pub fn row(report: &Reconciled) -> (PermissionStatus, String, HelperPermissionPhase) { let status = match report { Reconciled::AtThisBuild => PermissionStatus::Granted, _ => PermissionStatus::Pending, }; - (status, describe(report)) + let phase = phase(report); + let detail = match phase { + HelperPermissionPhase::Reconciling => FINISHING_SETUP.to_string(), + HelperPermissionPhase::Failed => HELPER_UPDATE_FAILED.to_string(), + HelperPermissionPhase::NeedsUserAction + if matches!(report, Reconciled::ServiceDefinitionBroken) => + { + SERVICE_DEFINITION_BROKEN.to_string() + } + _ => describe(report), + }; + (status, detail, phase) +} + +/// Friendly detail for helper actions returned outside the permission row. +/// Internal callers that need exact diagnostics continue to use [`describe`]. +pub fn action_detail(report: &Reconciled) -> String { + match report { + Reconciled::Stopped(_) => ACTION_FAILED.to_string(), + Reconciled::Busy => FINISHING_SETUP.to_string(), + Reconciled::ServiceDefinitionBroken => SERVICE_DEFINITION_BROKEN.to_string(), + _ => describe(report), + } +} + +/// Typed product state for every reconciliation report. +pub fn phase(report: &Reconciled) -> HelperPermissionPhase { + phase_with_failure_visibility(report, PRESENTATION.failure_visible()) +} + +fn phase_with_failure_visibility( + report: &Reconciled, + stopped_failure_visible: bool, +) -> HelperPermissionPhase { + match report { + Reconciled::AtThisBuild => HelperPermissionPhase::Ready, + Reconciled::PendingApproval => HelperPermissionPhase::ApprovalRequired, + Reconciled::NoHelper | Reconciled::Removed => HelperPermissionPhase::Disabled, + Reconciled::Busy => { + if stopped_failure_visible { + HelperPermissionPhase::Failed + } else { + HelperPermissionPhase::Reconciling + } + } + Reconciled::WaitingOnActivation(_) => HelperPermissionPhase::WaitingForActivation, + Reconciled::Displaced(_) | Reconciled::ServiceDefinitionBroken => { + HelperPermissionPhase::NeedsUserAction + } + Reconciled::Stopped(_) => { + if stopped_failure_visible { + HelperPermissionPhase::Failed + } else { + HelperPermissionPhase::Reconciling + } + } + } } /// Whether a pane of System Settings is where the user finishes this report. @@ -201,6 +297,11 @@ const FAST_ATTEMPTS: u32 = 30; /// glance. const WAITING_INTERVAL: Duration = Duration::from_secs(5); +/// How long retryable reports stay a neutral setup state before the UI offers +/// recovery. This is presentation timing only; the independent safety bound +/// below still governs how many mutations the loop may attempt. +const FAILURE_PRESENTATION_AFTER: Duration = Duration::from_secs(10); + /// Working attempts one loop makes before giving up and leaving repair to the /// next launch or the next click. /// @@ -209,6 +310,95 @@ const WAITING_INTERVAL: Duration = Duration::from_secs(5); /// that could churn a registration, and nothing else. const MAX_ATTEMPTS: u32 = 100; +/// Shared presentation state for explicit attempts and every row producer. +/// A Retry starts a new display window without resetting the loop's mutation +/// bound. Once its own pass fails, only another attempt or a settled observation +/// clears that failure; the background loop must not reset it again. +/// The generation and visibility change under one lock, so an older observation +/// cannot clear a newer failure between a generation check and its update. +/// No reconciliation, publication, or other external call runs under this lock. +#[derive(Default)] +struct FailurePresentation { + state: Mutex, +} + +#[derive(Default)] +struct FailurePresentationState { + generation: u64, + failure_visible: bool, + last_failure_detail: Option, +} + +static PRESENTATION: FailurePresentation = FailurePresentation { + state: Mutex::new(FailurePresentationState { + generation: 0, + failure_visible: false, + last_failure_detail: None, + }), +}; + +/// Last observed helper failure for the existing opt-in feedback snapshot. +/// Reading it never probes permissions or changes helper registration. +pub(crate) fn last_failure_detail() -> Option { + PRESENTATION.last_failure_detail() +} + +impl FailurePresentation { + fn generation(&self) -> u64 { + self.state + .lock() + .unwrap_or_else(PoisonError::into_inner) + .generation + } + + fn failure_visible(&self) -> bool { + self.state + .lock() + .unwrap_or_else(PoisonError::into_inner) + .failure_visible + } + + fn last_failure_detail(&self) -> Option { + self.state + .lock() + .unwrap_or_else(PoisonError::into_inner) + .last_failure_detail + .clone() + } + + fn begin_user_attempt(&self) -> u64 { + let mut state = self.state.lock().unwrap_or_else(PoisonError::into_inner); + state.generation = state.generation.wrapping_add(1); + state.failure_visible = false; + state.generation + } + + fn show_failure(&self, generation: u64) { + let mut state = self.state.lock().unwrap_or_else(PoisonError::into_inner); + if state.generation == generation { + state.failure_visible = true; + } + } + + fn note_settled_report(&self, report: &Reconciled, generation: u64) { + let mut state = self.state.lock().unwrap_or_else(PoisonError::into_inner); + if state.generation != generation || matches!(report, Reconciled::Busy) { + return; + } + state.last_failure_detail = match report { + Reconciled::Stopped(_) => Some(describe(report)), + _ => None, + }; + if !matches!(report, Reconciled::Stopped(_)) { + state.failure_visible = false; + } + } +} + +fn should_show_failure(elapsed: Duration, working: u32) -> bool { + elapsed >= FAILURE_PRESENTATION_AFTER || working >= MAX_ATTEMPTS +} + /// Whether re-running could still change anything. fn nothing_more_to_do(report: &Reconciled) -> bool { matches!( @@ -279,23 +469,72 @@ pub fn start_converging(app: &AppHandle) { std::thread::spawn(move || { let _guard = guard; drive( - || { - let report = observe(&app); - permissions_state::replace_row(&app, permissions::helper_row(&report)); - report + &PRESENTATION, + || observe(&app), + |report| { + permissions_state::replace_row(&app, permissions::helper_row(report)); }, std::thread::sleep, ); }); } -/// Run, publish, stop when there is nothing left to do, otherwise wait and go -/// again. -fn drive(mut attempt: impl FnMut() -> Reconciled, mut wait: impl FnMut(Duration)) { +/// Run, publish one shared product state, and stop when there is nothing left +/// to do. Retry timing lives here so full refreshes and both UI surfaces read +/// the same phase rather than independently interpreting transient reports. +fn drive( + presentation: &FailurePresentation, + attempt: impl FnMut() -> Reconciled, + publish: impl FnMut(&Reconciled), + wait: impl FnMut(Duration), +) { + drive_with_clock(presentation, attempt, publish, wait, Instant::now); +} + +fn drive_with_clock( + presentation: &FailurePresentation, + mut attempt: impl FnMut() -> Reconciled, + mut publish: impl FnMut(&Reconciled), + mut wait: impl FnMut(Duration), + mut now: impl FnMut() -> Instant, +) { + // `working` remains solely the mutation-safety bound and cadence selector. + // Presentation timing is deliberately independent so a human approval or + // a long activation wait cannot reset the churn limit. let mut working = 0; + let mut user_generation = presentation.generation(); + let mut presentation_started: Option = None; loop { + let observed_generation = presentation.generation(); + if observed_generation != user_generation { + user_generation = observed_generation; + presentation_started = Some(now()); + } else if presentation_started.is_none() && !presentation.failure_visible() { + presentation_started = Some(now()); + } + + let attempt_generation = presentation.generation(); let report = attempt(); + + // A user action can begin while this loop's pass owns RUNNING; that + // action waits for the pass. Do not let the older pass restore a stale + // failure after the new attempt has already reset its presentation. + let generation_after_attempt = presentation.generation(); + if generation_after_attempt != user_generation { + user_generation = generation_after_attempt; + presentation_started = Some(now()); + } + + // Busy says only that this observation lost the single-flight race. It + // must not replace an approval, failure, or actionable instruction. + if matches!(report, Reconciled::Busy) { + wait(WAITING_INTERVAL); + continue; + } + if nothing_more_to_do(&report) { + presentation.note_settled_report(&report, attempt_generation); + publish(&report); return; } // None of these attempted anything. `PendingApproval` is macOS @@ -320,12 +559,28 @@ fn drive(mut attempt: impl FnMut() -> Reconciled, mut wait: impl FnMut(Duration) // Login Items. Every such cycle is therefore paced by a human. if matches!( report, - Reconciled::PendingApproval | Reconciled::Busy | Reconciled::WaitingOnActivation(_) + Reconciled::PendingApproval | Reconciled::WaitingOnActivation(_) ) { + // This is a genuine wait boundary, not another failed mutation. + // Give the work after it a fresh *display* window while leaving the + // lifetime `working` safety counter untouched. + presentation_started = None; + presentation.note_settled_report(&report, attempt_generation); + publish(&report); wait(WAITING_INTERVAL); continue; } + + debug_assert!(matches!(report, Reconciled::Stopped(_))); working += 1; + let elapsed = presentation_started + .map(|started| now().saturating_duration_since(started)) + .unwrap_or_default(); + if should_show_failure(elapsed, working) { + presentation.show_failure(attempt_generation); + } + publish(&report); + if working >= MAX_ATTEMPTS { log::warn!( "helper convergence: giving up after {working} attempts, last report {report:?}; \ @@ -420,11 +675,210 @@ mod tests { } } + #[test] + fn every_report_has_a_typed_product_phase() { + for (report, expected) in [ + (Reconciled::AtThisBuild, HelperPermissionPhase::Ready), + ( + Reconciled::PendingApproval, + HelperPermissionPhase::ApprovalRequired, + ), + (Reconciled::NoHelper, HelperPermissionPhase::Disabled), + (Reconciled::Removed, HelperPermissionPhase::Disabled), + ( + Reconciled::WaitingOnActivation(None), + HelperPermissionPhase::WaitingForActivation, + ), + (displaced(), HelperPermissionPhase::NeedsUserAction), + ( + Reconciled::ServiceDefinitionBroken, + HelperPermissionPhase::NeedsUserAction, + ), + ] { + assert_eq!(phase_with_failure_visibility(&report, false), expected); + } + + let stopped = stopped("SMAppServiceErrorDomain 1: operation not permitted"); + assert_eq!( + phase_with_failure_visibility(&stopped, false), + HelperPermissionPhase::Reconciling + ); + assert_eq!( + phase_with_failure_visibility(&stopped, true), + HelperPermissionPhase::Failed + ); + } + + #[test] + fn technical_stops_are_friendly_only_at_the_row_boundary() { + let report = stopped("SMAppServiceErrorDomain 1: operation not permitted"); + assert!(describe(&report).contains("SMAppServiceErrorDomain 1")); + assert!(!action_detail(&report).contains("SMAppServiceErrorDomain")); + } + + #[test] + fn presentation_uses_time_without_changing_the_mutation_bound() { + assert!(!should_show_failure(Duration::from_secs(9), 99)); + assert!(should_show_failure(FAILURE_PRESENTATION_AFTER, 1)); + assert!(should_show_failure(Duration::ZERO, MAX_ATTEMPTS)); + } + + #[test] + fn feedback_keeps_failure_details_without_changing_friendly_copy() { + let presentation = FailurePresentation::default(); + let first = presentation.begin_user_attempt(); + let failure = stopped("SMAppServiceErrorDomain 1: registration refused"); + presentation.note_settled_report(&failure, first); + assert!( + presentation + .last_failure_detail() + .unwrap() + .contains("SMAppServiceErrorDomain 1") + ); + assert!(!action_detail(&failure).contains("SMAppServiceErrorDomain")); + presentation.note_settled_report(&Reconciled::Busy, first); + assert!(presentation.last_failure_detail().is_some()); + + let next = presentation.begin_user_attempt(); + presentation.note_settled_report(&Reconciled::AtThisBuild, first); + assert!(presentation.last_failure_detail().is_some()); + presentation.note_settled_report(&Reconciled::AtThisBuild, next); + assert!(presentation.last_failure_detail().is_none()); + } + + #[test] + fn a_completed_retry_failure_survives_the_background_generation_check() { + let presentation = FailurePresentation::default(); + let mut attempts = 0; + let mut phases = Vec::new(); + drive( + &presentation, + || { + attempts += 1; + if attempts == 1 { + // The background pass releases reconciliation's slot, then + // Retry runs and fails before this loop publishes its report. + let retry_generation = presentation.begin_user_attempt(); + presentation.show_failure(retry_generation); + stopped("the helper could not be registered") + } else { + Reconciled::AtThisBuild + } + }, + |report| { + phases.push(phase_with_failure_visibility( + report, + presentation.failure_visible(), + )); + }, + |_| {}, + ); + assert_eq!( + phases, + vec![HelperPermissionPhase::Failed, HelperPermissionPhase::Ready] + ); + } + + #[test] + fn a_stale_settled_observation_cannot_clear_a_newer_failed_retry() { + let presentation = FailurePresentation::default(); + drive( + &presentation, + || { + observe_with(&presentation, || { + // This observation already obtained Ready and released the + // reconciliation slot. Retry acquires it and fails before + // the older observation returns to its presentation code. + let retry_generation = presentation.begin_user_attempt(); + presentation.show_failure(retry_generation); + Reconciled::AtThisBuild + }) + }, + |_| { + // Both observe() and drive() handle the old settled result + // before this publisher runs. Neither may clear Retry's latch. + assert_eq!( + phase_with_failure_visibility( + &stopped("retry failed"), + presentation.failure_visible(), + ), + HelperPermissionPhase::Failed, + ); + }, + |_| {}, + ); + assert!(presentation.failure_visible()); + } + + #[test] + fn the_loop_shows_recovery_after_the_display_window() { + let presentation = FailurePresentation::default(); + let started = Instant::now(); + let elapsed = std::cell::Cell::new(Duration::ZERO); + let mut attempts = 0; + let mut phases = Vec::new(); + drive_with_clock( + &presentation, + || { + attempts += 1; + if attempts <= 3 { + stopped("the helper could not be registered") + } else { + Reconciled::AtThisBuild + } + }, + |report| { + phases.push(phase_with_failure_visibility( + report, + presentation.failure_visible(), + )); + }, + |_| { + elapsed.set(if elapsed.get().is_zero() { + Duration::from_secs(9) + } else { + elapsed.get() + Duration::from_secs(1) + }); + }, + || started + elapsed.get(), + ); + assert_eq!( + phases, + vec![ + HelperPermissionPhase::Reconciling, + HelperPermissionPhase::Reconciling, + HelperPermissionPhase::Failed, + HelperPermissionPhase::Ready, + ] + ); + } + + #[test] + fn busy_never_replaces_the_last_truthful_row() { + let mut attempts = 0; + let mut published = Vec::new(); + drive( + &FailurePresentation::default(), + || { + attempts += 1; + if attempts == 1 { + Reconciled::Busy + } else { + Reconciled::AtThisBuild + } + }, + |report| published.push(report.clone()), + |_| {}, + ); + assert_eq!(published, vec![Reconciled::AtThisBuild]); + } + #[test] fn it_stops_as_soon_as_the_decision_is_carried_out() { let mut attempts = 0; let mut waited = Vec::new(); drive( + &FailurePresentation::default(), || { attempts += 1; if attempts < 3 { @@ -433,6 +887,7 @@ mod tests { Reconciled::AtThisBuild } }, + |_| {}, |interval| waited.push(interval), ); assert_eq!(attempts, 3); @@ -448,6 +903,7 @@ mod tests { let mut attempts = 0; let mut waited = Vec::new(); drive( + &FailurePresentation::default(), || { attempts += 1; if attempts <= MAX_ATTEMPTS * 10 { @@ -456,6 +912,7 @@ mod tests { Reconciled::AtThisBuild } }, + |_| {}, |interval| waited.push(interval), ); assert_eq!(attempts, MAX_ATTEMPTS * 10 + 1, "the loop stopped watching"); @@ -471,6 +928,7 @@ mod tests { let mut attempts = 0; let mut working = 0; drive( + &FailurePresentation::default(), || { attempts += 1; if attempts <= 500 { @@ -481,6 +939,7 @@ mod tests { } }, |_| {}, + |_| {}, ); assert_eq!(working, MAX_ATTEMPTS); } @@ -490,10 +949,12 @@ mod tests { let mut attempts = 0; let mut waited = Vec::new(); drive( + &FailurePresentation::default(), || { attempts += 1; stopped("the helper could not be registered: not permitted") }, + |_| {}, |interval| waited.push(interval), ); assert_eq!(attempts, MAX_ATTEMPTS); diff --git a/apps/native/src-tauri/src/system/permissions.rs b/apps/native/src-tauri/src/system/permissions.rs index 896ba0ded..42370f120 100644 --- a/apps/native/src-tauri/src/system/permissions.rs +++ b/apps/native/src-tauri/src/system/permissions.rs @@ -11,7 +11,9 @@ //! - Administrator privileges (sudo access) use crate::privileged_helper::reconcile::Reconciled; +use crate::shared_types::HelperPermissionPhase; pub(crate) use crate::shared_types::{Permission, PermissionStatus, PermissionsState}; +use crate::state::permissions_state; use crate::system::helper_permission; use anyhow::Result; use log::{debug, info, warn}; @@ -48,6 +50,7 @@ fn get_default_permissions() -> Vec { can_request_programmatically: true, status: default_status, instructions: None, + helper_phase: None, }, Permission { id: "documents".to_string(), @@ -58,6 +61,7 @@ fn get_default_permissions() -> Vec { can_request_programmatically: true, status: default_status, instructions: None, + helper_phase: None, }, Permission { id: "admin".to_string(), @@ -68,6 +72,7 @@ fn get_default_permissions() -> Vec { can_request_programmatically: false, status: default_status, instructions: Some("You will be prompted for your password when needed".to_string()), + helper_phase: None, }, Permission { id: "full-disk".to_string(), @@ -80,6 +85,7 @@ fn get_default_permissions() -> Vec { "First make sure nixmac is in your Applications folder (not running from the install disk image). Then go to System Settings → Privacy & Security → Full Disk Access and add nixmac to the list." .to_string(), ), + helper_phase: None, }, app_management_permission(PermissionStatus::Pending), privileged_helper_permission( @@ -88,6 +94,7 @@ fn get_default_permissions() -> Vec { // No report yet, so nothing says macOS is waiting on the user. Every // path that has one replaces this row whole (`helper_row`). true, + Some(HelperPermissionPhase::Disabled), ), ] } @@ -125,6 +132,9 @@ fn granted_permissions_state() -> PermissionsState { let mut permissions = get_default_permissions(); for perm in &mut permissions { perm.status = PermissionStatus::Granted; + if perm.id == "privileged-helper" { + perm.helper_phase = Some(HelperPermissionPhase::Ready); + } // Granted here is a debug-build fiction, not a probe result; say so // instead of letting the row imply the real thing was verified. perm.instructions = Some( @@ -148,6 +158,7 @@ fn granted_folder_permission(id: &str, name: &str, description: &str) -> Permiss can_request_programmatically: true, status: PermissionStatus::Granted, instructions: None, + helper_phase: None, } } @@ -155,6 +166,7 @@ fn privileged_helper_permission( status: PermissionStatus, instructions: &str, can_request_programmatically: bool, + helper_phase: Option, ) -> Permission { Permission { id: "privileged-helper".to_string(), @@ -166,6 +178,7 @@ fn privileged_helper_permission( can_request_programmatically, status, instructions: Some(instructions.to_string()), + helper_phase, } } @@ -202,6 +215,7 @@ fn app_management_permission(status: PermissionStatus) -> Permission { "Open System Settings → Privacy & Security → App Management and enable nixmac. macOS does not expose a reliable way for nixmac to verify this grant." .to_string(), ), + helper_phase: None, } } @@ -412,7 +426,7 @@ pub fn check_all_permissions(app: &AppHandle) -> PermissionsState // get anywhere on its own — so it is replaced rather than patched field by // field, and `helper_row` stays the only place a report becomes a row. if perm.id == "privileged-helper" { - *perm = helper_row(&check_privileged_helper(app)); + *perm = check_privileged_helper(app); } else { perm.status = match perm.id.as_str() { "desktop" => check_desktop_access(), @@ -509,6 +523,7 @@ pub fn request_permission( can_request_programmatically: true, status: PermissionStatus::Granted, instructions: None, + helper_phase: None, }) } Err(_) => Ok(Permission { @@ -520,6 +535,7 @@ pub fn request_permission( can_request_programmatically: true, status: PermissionStatus::Denied, instructions: None, + helper_phase: None, }), } } @@ -551,6 +567,7 @@ pub fn request_permission( can_request_programmatically: true, status: PermissionStatus::Granted, instructions: None, + helper_phase: None, }) } Err(_) => Ok(Permission { @@ -563,6 +580,7 @@ pub fn request_permission( can_request_programmatically: true, status: PermissionStatus::Denied, instructions: None, + helper_phase: None, }), } } @@ -579,6 +597,7 @@ pub fn request_permission( instructions: Some( "You will be prompted for your password when needed".to_string(), ), + helper_phase: None, }) } "full-disk" => { @@ -602,6 +621,7 @@ pub fn request_permission( "First make sure nixmac is in your Applications folder (not running from the install disk image). Then go to System Settings → Privacy & Security → Full Disk Access and add nixmac to the list." .to_string() })), + helper_phase: None, }) } "app-management" => { @@ -635,7 +655,7 @@ pub fn request_permission( /// a report, and the report is what tells the user what to do: approve in Login /// Items, move the app to /Applications, restart the app, or wait out a running /// activation. -fn check_privileged_helper(app: &AppHandle) -> Reconciled { +fn check_privileged_helper(app: &AppHandle) -> Permission { let report = helper_permission::observe(app); // A refresh is how a user comes back to this — opening the panel, or // pressing Check again — and one run is usually not enough: the platform @@ -644,16 +664,35 @@ fn check_privileged_helper(app: &AppHandle) -> Reconciled { // Starting the loop here means the visit converges instead of handing back a // scary report. A loop already running ignores this. helper_permission::start_converging(app); - report + let previous = permissions_state::get(app).and_then(|state| { + state + .permissions + .into_iter() + .find(|permission| permission.id == "privileged-helper") + }); + helper_row_from_observation(&report, previous) +} + +fn helper_row_from_observation(report: &Reconciled, previous: Option) -> Permission { + // Busy means this observation learned nothing. Preserve the last truthful + // helper row rather than downgrading approval, failure, or an actionable + // instruction to a generic progress state. + if matches!(report, Reconciled::Busy) + && let Some(row) = previous + { + return row; + } + helper_row(report) } /// One report as the permission row it produces. pub(crate) fn helper_row(report: &Reconciled) -> Permission { - let (status, detail) = helper_permission::row(report); + let (status, detail, phase) = helper_permission::row(report); privileged_helper_permission( status, &detail, helper_permission::can_request_programmatically(report), + Some(phase), ) } @@ -681,9 +720,33 @@ mod tests { /// them is what the UI actually reads and nothing else pins it. #[test] fn the_row_takes_its_deep_link_from_the_report() { - assert!(!helper_row(&Reconciled::PendingApproval).can_request_programmatically); - assert!(helper_row(&Reconciled::AtThisBuild).can_request_programmatically); - assert!(helper_row(&Reconciled::NoHelper).can_request_programmatically); + let approval = helper_row(&Reconciled::PendingApproval); + assert!(!approval.can_request_programmatically); + assert_eq!( + approval.helper_phase, + Some(HelperPermissionPhase::ApprovalRequired) + ); + + let ready = helper_row(&Reconciled::AtThisBuild); + assert!(ready.can_request_programmatically); + assert_eq!(ready.helper_phase, Some(HelperPermissionPhase::Ready)); + + let disabled = helper_row(&Reconciled::NoHelper); + assert!(disabled.can_request_programmatically); + assert_eq!(disabled.helper_phase, Some(HelperPermissionPhase::Disabled)); + } + + #[test] + fn a_busy_full_refresh_preserves_the_previous_helper_row() { + let previous = helper_row(&Reconciled::PendingApproval); + assert_eq!( + helper_row_from_observation(&Reconciled::Busy, Some(previous.clone())).helper_phase, + previous.helper_phase + ); + assert_eq!( + helper_row_from_observation(&Reconciled::Busy, Some(previous.clone())).instructions, + previous.instructions + ); } /// A bare app handle. The skip flags below short-circuit before anything diff --git a/apps/native/src-tauri/src/types.rs b/apps/native/src-tauri/src/types.rs index 13e6095a9..c83f4d601 100644 --- a/apps/native/src-tauri/src/types.rs +++ b/apps/native/src-tauri/src/types.rs @@ -589,26 +589,68 @@ pub(crate) fn emit_evolve_event(app: &tauri::AppHandle, ev } if let Some(window) = app.get_webview_window("main") { - // A question blocks the run until the user answers; nudge them if - // they are looking elsewhere. - if matches!(event.event_type, EvolveEventType::Question) - && !window.is_focused().unwrap_or(false) - { - notify_question(app, &window, &event.summary); - } + let attention = evolve_event_attention(&event.event_type, crate::main_window::active(app)); if let Err(e) = tauri::Emitter::emit(&window, EVOLVE_EVENT_CHANNEL, &event) { log::warn!("Failed to emit evolve event: {}", e); } + match attention { + EvolveEventAttention::None => {} + EvolveEventAttention::InputRequired => crate::attention::input_required(app), + EvolveEventAttention::QuestionNotificationIfUnfocused => { + notify_question(app, &window, &event.summary); + } + EvolveEventAttention::ClearWork => crate::attention::clear_work(app), + } + } +} + +/// What an evolve event asks of native attention. Decided from the event type +/// and window mode alone, so the popover-versus-control routing is testable +/// without a running app. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum EvolveEventAttention { + None, + /// The popover's own notification backend posts the input-required + /// notice, whether or not the window is focused, because the popover + /// owns the process-wide macOS notification delegate. + InputRequired, + /// Control-window mode: request attention and post a plugin + /// notification, but only while the window is unfocused. + QuestionNotificationIfUnfocused, + /// A question was answered in the popover: retire its pending notice. + ClearWork, +} + +fn evolve_event_attention( + event_type: &EvolveEventType, + mode: crate::main_window::MainWindowMode, +) -> EvolveEventAttention { + match event_type { + EvolveEventType::Question + if crate::attention::plugin_notification_backend_allowed_for_mode( + mode.is_popover(), + ) => + { + EvolveEventAttention::QuestionNotificationIfUnfocused + } + EvolveEventType::Question => EvolveEventAttention::InputRequired, + EvolveEventType::Answered if mode.is_popover() => EvolveEventAttention::ClearWork, + _ => EvolveEventAttention::None, } } -/// OS-level nudge for a question that arrived while the app was unfocused: -/// a notification plus a request for attention (dock bounce on macOS). +/// OS-level nudge for a question that arrived while the control window was +/// unfocused: a notification plus a request for attention (dock bounce on +/// macOS). fn notify_question( app: &tauri::AppHandle, window: &tauri::WebviewWindow, question: &str, ) { + if window.is_focused().unwrap_or(false) { + return; + } + use tauri_plugin_notification::NotificationExt; if let Err(e) = window.request_user_attention(Some(tauri::UserAttentionType::Informational)) { @@ -629,6 +671,66 @@ fn notify_question( mod tests { use super::*; + mod evolve_event_attention_routing { + use super::super::{EvolveEventAttention, evolve_event_attention}; + use crate::main_window::MainWindowMode; + use crate::shared_types::EvolveEventType; + + #[test] + fn a_question_in_the_control_window_uses_the_unfocused_plugin_notification() { + assert_eq!( + evolve_event_attention(&EvolveEventType::Question, MainWindowMode::Control), + EvolveEventAttention::QuestionNotificationIfUnfocused + ); + } + + #[test] + fn a_question_in_the_popover_goes_through_the_owned_attention_backend() { + let expected = if cfg!(target_os = "macos") { + EvolveEventAttention::InputRequired + } else { + // Off macOS there is no shared delegate to protect, so the + // plugin path stays available in both modes. + EvolveEventAttention::QuestionNotificationIfUnfocused + }; + assert_eq!( + evolve_event_attention(&EvolveEventType::Question, MainWindowMode::Popover), + expected + ); + } + + #[test] + fn an_answer_clears_pending_work_attention_only_in_the_popover() { + assert_eq!( + evolve_event_attention(&EvolveEventType::Answered, MainWindowMode::Popover), + EvolveEventAttention::ClearWork + ); + assert_eq!( + evolve_event_attention(&EvolveEventType::Answered, MainWindowMode::Control), + EvolveEventAttention::None + ); + } + + #[test] + fn other_events_never_route_attention_in_either_mode() { + for mode in [MainWindowMode::Control, MainWindowMode::Popover] { + for event_type in [ + EvolveEventType::Start, + EvolveEventType::Thinking, + EvolveEventType::Complete, + EvolveEventType::Error, + EvolveEventType::Info, + ] { + assert_eq!( + evolve_event_attention(&event_type, mode), + EvolveEventAttention::None, + "{event_type:?} in {mode:?}" + ); + } + } + } + } + #[test] fn thinking_summary_should_be_first_sentence_of_thought() { let event = EvolveEvent::thinking( diff --git a/apps/native/src/components/widget/onboarding/celebration-overlay.test.tsx b/apps/native/src/components/widget/onboarding/celebration-overlay.test.tsx new file mode 100644 index 000000000..e18faf780 --- /dev/null +++ b/apps/native/src/components/widget/onboarding/celebration-overlay.test.tsx @@ -0,0 +1,140 @@ +import { + Dialog, + DialogContent, + DialogDescription, + DialogTitle, +} from "@/components/ui/dialog"; +import { + ESCAPE_OWNER_PRIORITY, + registerEscapeOwner, + routeEscapeToOwner, +} from "@/lib/escape-owner"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { useEffect, useState } from "react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { CelebrationOverlay } from "./celebration-overlay"; + +vi.mock("lottie-react", () => ({ + default: () => null, +})); + +function EscapeDispatcher({ onPopover }: { onPopover: () => void }) { + useEffect(() => { + const unregisterPopover = registerEscapeOwner({ + name: "test-popover", + priority: ESCAPE_OWNER_PRIORITY.popover, + handle: () => { + onPopover(); + return true; + }, + }); + const dispatch = (event: KeyboardEvent) => routeEscapeToOwner(event); + window.addEventListener("keydown", dispatch); + + return () => { + window.removeEventListener("keydown", dispatch); + unregisterPopover(); + }; + }, [onPopover]); + + return null; +} + +function dispatchEscape(init: KeyboardEventInit = {}) { + const event = new KeyboardEvent("keydown", { + key: "Escape", + bubbles: true, + cancelable: true, + ...init, + }); + fireEvent(document, event); + return event; +} + +describe("CelebrationOverlay", () => { + beforeEach(() => { + const json = vi.fn<() => Promise>().mockResolvedValue({}); + vi.stubGlobal( + "fetch", + vi.fn().mockResolvedValue({ json } as unknown as Response), + ); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("orders Radix, celebration, then popover across consecutive Escapes", async () => { + const onCelebrationDismiss = vi.fn<() => void>(); + const onPopoverDismiss = vi.fn<() => void>(); + + function EscapeStack() { + const [dialogOpen, setDialogOpen] = useState(true); + const [celebrating, setCelebrating] = useState(true); + + return ( + <> + + {celebrating ? ( + { + onCelebrationDismiss(); + setCelebrating(false); + }} + /> + ) : null} + + + Blocking confirmation + Resolve this before closing the celebration. + + + + ); + } + + render(); + + dispatchEscape(); + await waitFor(() => + expect(screen.queryByRole("dialog", { name: "Blocking confirmation" })).toBeNull(), + ); + expect(onCelebrationDismiss).not.toHaveBeenCalled(); + expect(onPopoverDismiss).not.toHaveBeenCalled(); + + const celebrationEscape = dispatchEscape(); + expect(celebrationEscape.defaultPrevented).toBe(true); + await waitFor(() => expect(onCelebrationDismiss).toHaveBeenCalledOnce()); + expect(onPopoverDismiss).not.toHaveBeenCalled(); + + const popoverEscape = dispatchEscape(); + expect(popoverEscape.defaultPrevented).toBe(true); + expect(onPopoverDismiss).toHaveBeenCalledOnce(); + }); + + it("leaves default-prevented and IME Escape untouched", () => { + const onDismiss = vi.fn<() => void>(); + const onPopoverDismiss = vi.fn<() => void>(); + render( + <> + + + , + ); + + const defaultPrevented = new KeyboardEvent("keydown", { + key: "Escape", + bubbles: true, + cancelable: true, + }); + defaultPrevented.preventDefault(); + fireEvent(document, defaultPrevented); + + dispatchEscape({ isComposing: true }); + + expect(onDismiss).not.toHaveBeenCalled(); + expect(onPopoverDismiss).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/native/src/components/widget/onboarding/celebration-overlay.tsx b/apps/native/src/components/widget/onboarding/celebration-overlay.tsx index 6e835f599..879abb1a1 100644 --- a/apps/native/src/components/widget/onboarding/celebration-overlay.tsx +++ b/apps/native/src/components/widget/onboarding/celebration-overlay.tsx @@ -4,6 +4,7 @@ import { Button } from "@/components/ui/button"; import Lottie, { type LottieRefCurrentProps } from "lottie-react"; import { ArrowRight } from "lucide-react"; import { useEffect, useRef, useState } from "react"; +import { ESCAPE_OWNER_PRIORITY, registerEscapeOwner } from "@/lib/escape-owner"; const CELEBRATION_ANIMATION_SPEED = 0.8; @@ -52,13 +53,18 @@ export function CelebrationOverlay({ host, onDismiss }: CelebrationOverlayProps) }; }, []); - // Allow Escape to dismiss. + // Radix dialogs get first refusal at document level. After that, the + // explicit owner priority keeps this full-window moment above widget-owned + // overlays and the popover itself without relying on listener order. useEffect(() => { - function onKey(e: KeyboardEvent) { - if (e.key === "Escape") onDismiss(); - } - window.addEventListener("keydown", onKey); - return () => window.removeEventListener("keydown", onKey); + return registerEscapeOwner({ + name: "onboarding-celebration", + priority: ESCAPE_OWNER_PRIORITY.celebration, + handle: () => { + onDismiss(); + return true; + }, + }); }, [onDismiss]); return ( diff --git a/apps/native/src/components/widget/permissions/permissions-panel.test.tsx b/apps/native/src/components/widget/permissions/permissions-panel.test.tsx index 2541d3d4e..f6b8d7dc2 100644 --- a/apps/native/src/components/widget/permissions/permissions-panel.test.tsx +++ b/apps/native/src/components/widget/permissions/permissions-panel.test.tsx @@ -6,22 +6,22 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; /** * The helper row is the one permission nixmac installs rather than asks macOS - * for, so it is the one row that can hand it back — as one button, a toggle of - * the standing decision. Everything else the row shows comes from the backend's - * reconciliation report: the status decides whether "Granted" is shown, and - * `instructions` is the sentence. + * for, so it is the one row that can hand it back. A persistent failure keeps + * that Disable escape hatch and adds a non-destructive Retry. Everything else + * comes from the backend's typed phase and customer-facing sentence. */ -const mockRefresh = vi.fn(); -const mockRequest = vi.fn(); -const mockDisableHelper = vi.fn(); +const mockRefresh = vi.fn<(...args: unknown[]) => Promise>(); +const mockRequest = vi.fn<(...args: unknown[]) => Promise>(); +const mockDisableHelper = vi.fn<(...args: unknown[]) => Promise>(); +const mockRetryHelper = vi.fn<(...args: unknown[]) => Promise>(); vi.mock("@/ipc/api", () => ({ tauriAPI: { permissions: { refresh: (...args: unknown[]) => mockRefresh(...args), request: (...args: unknown[]) => mockRequest(...args), - requestFullDiskAccess: vi.fn(), + requestFullDiskAccess: vi.fn<() => void>(), }, }, })); @@ -30,6 +30,7 @@ vi.mock("@/lib/orpc", () => ({ client: { darwin: { helperDisable: (...args: unknown[]) => mockDisableHelper(...args), + helperRetry: (...args: unknown[]) => mockRetryHelper(...args), }, permissions: { refresh: (...args: unknown[]) => mockRefresh(...args), @@ -47,8 +48,8 @@ vi.mock("@/lib/orpc", () => ({ }, })); -const permissionsState = vi.fn(); -const helperPreference = vi.fn(); +const permissionsState = vi.fn<() => unknown>(); +const helperPreference = vi.fn<() => string>(); vi.mock("@nixmac/state", () => ({ useViewModel: (select: (state: { permissions: unknown; preferences: unknown }) => unknown) => select({ @@ -70,6 +71,7 @@ function helperPermission(overrides: Partial) { canRequestProgrammatically: true, status: "pending", instructions: "a report", + helperPhase: "reconciling", ...overrides, }; } @@ -93,6 +95,7 @@ function awaitingApprovalRow() { helperPermission({ instructions: APPROVE_IN_LOGIN_ITEMS, canRequestProgrammatically: false, + helperPhase: "approvalRequired", }), ], allRequiredGranted: false, @@ -169,6 +172,99 @@ describe("PermissionsPanel — the unattended sync helper row", () => { } }); + it("offers Retry without removing Disable after automatic recovery fails", async () => { + helperPreference.mockReturnValue("granted"); + permissionsState.mockReturnValue({ + permissions: [ + helperPermission({ + helperPhase: "failed", + instructions: "nixmac couldn’t finish enabling unattended sync. Try again.", + }), + ], + allRequiredGranted: false, + checkedAt: null, + }); + mockRetryHelper.mockResolvedValue({ + atThisBuild: false, + phase: "reconciling", + detail: "nixmac is finishing unattended sync setup.", + }); + + const { getByRole } = await panel(); + expect(getByRole("button", { name: "Retry" })).toBeTruthy(); + expect(getByRole("button", { name: "Disable" })).toBeTruthy(); + + fireEvent.click(getByRole("button", { name: "Retry" })); + await waitFor(() => { + expect(mockRetryHelper).toHaveBeenCalledTimes(1); + expect(mockRefresh).toHaveBeenCalled(); + }); + expect(mockRequest).not.toHaveBeenCalled(); + }); + + it.each(["Retry", "Disable"] as const)( + "keeps %s as the only helper action until it finishes", + async (action) => { + helperPreference.mockReturnValue("granted"); + permissionsState.mockReturnValue({ + permissions: [helperPermission({ helperPhase: "failed" })], + allRequiredGranted: false, + checkedAt: null, + }); + let finish: () => void = () => {}; + const pending = new Promise((resolve) => { + finish = () => resolve({ detail: "Finished." }); + }); + (action === "Retry" ? mockRetryHelper : mockDisableHelper).mockReturnValue(pending); + + const { repaint } = await panel(); + fireEvent.click(screen.getByRole("button", { name: action })); + + const busyLabel = action === "Retry" ? "Retrying…" : "Disabling…"; + expect(screen.getByRole("button", { name: busyLabel })).toBeDisabled(); + expect(screen.queryByRole("button", { name: "Retry" })).toBeNull(); + expect(screen.queryByRole("button", { name: "Disable" })).toBeNull(); + + // Reconciliation publishes progress before the request settles. + permissionsState.mockReturnValue({ + permissions: [helperPermission({ helperPhase: "reconciling" })], + allRequiredGranted: false, + checkedAt: null, + }); + repaint(); + expect(screen.getByRole("button", { name: busyLabel })).toBeDisabled(); + expect(screen.queryByRole("button", { name: "Enable" })).toBeNull(); + + finish(); + await waitFor(() => { + expect(screen.queryByRole("button", { name: busyLabel })).toBeNull(); + expect(screen.getByRole("button", { name: "Disable" })).toBeEnabled(); + }); + }, + ); + + it("retries a failed removal without offering to enable the helper again", async () => { + helperPreference.mockReturnValue("disabled"); + permissionsState.mockReturnValue({ + permissions: [helperPermission({ helperPhase: "failed" })], + allRequiredGranted: false, + checkedAt: null, + }); + mockRetryHelper.mockResolvedValue({ phase: "disabled" }); + + await panel(); + expect(screen.queryByRole("button", { name: "Enable" })).toBeNull(); + expect(screen.queryByRole("button", { name: "Disable" })).toBeNull(); + fireEvent.click(screen.getByRole("button", { name: "Retry" })); + + await waitFor(() => { + expect(mockRetryHelper).toHaveBeenCalledOnce(); + expect(screen.getByRole("button", { name: "Retry" })).toBeEnabled(); + }); + expect(mockRequest).not.toHaveBeenCalled(); + expect(mockDisableHelper).not.toHaveBeenCalled(); + }); + it("offers the Login Items deep link, not Disable, while approval is pending", async () => { // macOS is holding the registration and nothing nixmac does next makes that // go, so the row offers the same "Open Settings" action as the other rows @@ -190,6 +286,25 @@ describe("PermissionsPanel — the unattended sync helper row", () => { } }); + it("does not offer an ineffective Enable for a helper that needs an app move or restart", async () => { + helperPreference.mockReturnValue("unset"); + permissionsState.mockReturnValue({ + permissions: [ + helperPermission({ + helperPhase: "needsUserAction", + instructions: "Move nixmac to /Applications and restart it.", + }), + ], + allRequiredGranted: false, + checkedAt: null, + }); + + await panel(); + + expect(screen.queryByRole("button", { name: "Enable" })).toBeNull(); + expect(screen.getByText("Move nixmac to /Applications and restart it.")).toBeTruthy(); + }); + it("opens Login Items through the same grant action the row always used", async () => { // The deep link is not a new backend call: it is `permissions.request` for // this row, which records the decision and reconciles, and is the only action @@ -210,6 +325,55 @@ describe("PermissionsPanel — the unattended sync helper row", () => { expect(mockDisableHelper).not.toHaveBeenCalled(); }); + it.each(["before", "after"] as const)( + "drops approval instructions when the helper becomes ready %s the grant response", + async (readyWhen) => { + permissionsState.mockReturnValue( + helperRow("pending", "The unattended sync helper is not installed."), + ); + let finishGrant: () => void = () => {}; + mockRequest.mockReturnValue( + new Promise((resolve) => { + finishGrant = () => resolve(awaitingApprovalRow().permissions[0]); + }), + ); + + const { repaint } = await panel(); + fireEvent.click(screen.getByRole("button", { name: "Enable" })); + helperPreference.mockReturnValue("granted"); + permissionsState.mockReturnValue(awaitingApprovalRow()); + repaint(); + + if (readyWhen === "after") { + finishGrant(); + await waitFor(() => expect(mockRefresh).toHaveBeenCalledTimes(2)); + expect(screen.getAllByText(APPROVE_IN_LOGIN_ITEMS)).toHaveLength(1); + } + + const ready = "The unattended sync helper is installed and answering."; + permissionsState.mockReturnValue({ + permissions: [ + helperPermission({ status: "granted", helperPhase: "ready", instructions: ready }), + ], + allRequiredGranted: true, + checkedAt: null, + }); + repaint(); + + if (readyWhen === "before") { + finishGrant(); + await waitFor(() => expect(mockRefresh).toHaveBeenCalledTimes(2)); + } + + await waitFor(() => + expect(screen.getByRole("button", { name: "Disable" })).toBeEnabled(), + ); + expect(screen.getByText(ready)).toBeTruthy(); + expect(screen.queryByText(APPROVE_IN_LOGIN_ITEMS)).toBeNull(); + expect(screen.queryByRole("button", { name: /Open Settings/ })).toBeNull(); + }, + ); + it("offers Disable on a granted row whose decision was never recorded", async () => { // An adopted registration: the reconciliation run records `granted` before // it reports, so this is the window before the mirrored preference catches @@ -223,7 +387,7 @@ describe("PermissionsPanel — the unattended sync helper row", () => { expect(screen.queryByRole("button", { name: "Enable" })).toBeNull(); }); - it("renders every report the backend can send for a row that is not granted", async () => { + it("renders actionable and friendly helper copy", async () => { // The vocabulary: move the app, restart it, wait out a running activation // (with and without the activation's details), and plain failures. Each // arrives as the row's instructions and is shown verbatim — the UI never @@ -237,7 +401,7 @@ describe("PermissionsPanel — the unattended sync helper row", () => { "this app was replaced while running (build a is running, b is installed) — restart nixmac.", "nixmac is waiting for a running activation to finish before updating the unattended sync helper (/nix/store/abc/activate submitted by the sync agent).", "nixmac is waiting for a running activation to finish before updating the unattended sync helper.", - "the helper could not be unregistered: SMAppService 1 refused.", + "nixmac couldn’t update the unattended sync helper. Try again. If this keeps happening, report the problem.", ]) { permissionsState.mockReturnValue(helperRow("pending", report)); @@ -329,20 +493,75 @@ describe("PermissionsPanel — the unattended sync helper row", () => { expect(running).toBeDisabled(); }); - it("disabling reports what the run did and re-probes", async () => { + it("uses the live row after disabling, without retaining a stale RPC report", async () => { permissionsState.mockReturnValue(helperRow("granted", "installed and answering")); mockDisableHelper.mockResolvedValue({ atThisBuild: false, - detail: "The unattended sync helper is disabled and has been removed.", + phase: "waitingForActivation", + detail: "nixmac is waiting for a running activation to finish before updating the unattended sync helper.", }); - const { getByRole } = await panel(); + const { getByRole, repaint } = await panel(); fireEvent.click(getByRole("button", { name: "Disable" })); await waitFor(() => { expect(mockDisableHelper).toHaveBeenCalledTimes(1); - expect(mockRefresh).toHaveBeenCalled(); + expect(mockRefresh).toHaveBeenCalledTimes(2); }); - expect(screen.getByText("The unattended sync helper is disabled and has been removed.")).toBeTruthy(); + const removed = "The unattended sync helper is disabled and has been removed."; + helperPreference.mockReturnValue("disabled"); + permissionsState.mockReturnValue({ + permissions: [helperPermission({ helperPhase: "disabled", instructions: removed })], + allRequiredGranted: false, + checkedAt: null, + }); + repaint(); + + expect(screen.getAllByText(removed)).toHaveLength(1); + expect(screen.queryByText(/waiting for a running activation/)).toBeNull(); + expect(screen.getByRole("button", { name: "Enable" })).toBeEnabled(); + }); + + it.each(["Enable", "Retry", "Disable"] as const)( + "preserves a thrown %s error as a notice", + async (action) => { + helperPreference.mockReturnValue(action === "Enable" ? "unset" : "granted"); + permissionsState.mockReturnValue({ + permissions: [helperPermission({ helperPhase: "failed" })], + allRequiredGranted: false, + checkedAt: null, + }); + const request = { + Enable: mockRequest, + Retry: mockRetryHelper, + Disable: mockDisableHelper, + }[action]; + request.mockRejectedValue(new Error("helper action transport failed")); + + await panel(); + fireEvent.click(screen.getByRole("button", { name: action })); + + await waitFor(() => expect(screen.getByText(/helper action transport failed/)).toBeTruthy()); + }, + ); + + it("preserves App Management guidance when the helper row changes", async () => { + const appManagement = { + ...adminRow, + id: "app-management", + name: "App Management", + instructions: "Allow nixmac to update managed apps.", + }; + permissionsState.mockReturnValue(helperRow("pending", "a report", appManagement)); + mockRequest.mockResolvedValue({}); + + const { repaint } = await panel(); + fireEvent.click(screen.getByRole("button", { name: /Open Settings/ })); + await waitFor(() => expect(screen.getByText(/nixmac opened System Settings/)).toBeTruthy()); + + permissionsState.mockReturnValue(helperRow("granted", "installed and answering", appManagement)); + repaint(); + expect(screen.getByText(/nixmac opened System Settings/)).toBeTruthy(); + await waitFor(() => expect(mockRefresh).toHaveBeenCalledTimes(2), { timeout: 2000 }); }); }); diff --git a/apps/native/src/components/widget/permissions/permissions-panel.tsx b/apps/native/src/components/widget/permissions/permissions-panel.tsx index 8bd97b1cb..9bb30297b 100644 --- a/apps/native/src/components/widget/permissions/permissions-panel.tsx +++ b/apps/native/src/components/widget/permissions/permissions-panel.tsx @@ -90,13 +90,15 @@ export function PermissionsPanel() { // The action in flight, per row. Per row because only the row that owns an // action is disabled; a single panel-wide slot would let a click on one row // evict another's entry while its action still runs. - const [requesting, setRequesting] = useState>( + const [requesting, setRequesting] = useState< + ReadonlyMap + >( () => new Map(), ); const [notice, setNotice] = useState<{ tone: "info" | "error"; message: string } | null>(null); // Updater form: two rows can start or settle in the same tick. - function startAction(id: string, action: "grant" | "disable") { + function startAction(id: string, action: "grant" | "retry" | "disable") { setRequesting((inFlight) => new Map(inFlight).set(id, action)); } @@ -143,21 +145,9 @@ export function PermissionsPanel() { "nixmac opened System Settings → Privacy & Security → App Management. Enable nixmac there, then return here. macOS does not let nixmac verify this permission, so this recommended row may remain pending.", }); await new Promise((resolve) => setTimeout(resolve, 1000)); - } else if (permission.id === HELPER_PERMISSION_ID) { - // The backend records the decision and reconciles; it may open Login - // Items when approval is pending. Show this run's report only when it - // differs from the row's sentence — otherwise every click while - // approval is pending prints the same sentence twice. - const result = await tauriAPI.permissions.request(permission.id); - if (result.status !== "granted" && result.instructions !== permission.instructions) { - setNotice({ - tone: "info", - message: - result.instructions ?? - "nixmac could not finish enabling the unattended sync helper.", - }); - } } else { + // Helper instructions come from the live permission row. Copying a + // request's result into a notice can outlive approval or recovery. // deprecated(orpc): replace with client/orpc from @/lib/orpc await tauriAPI.permissions.request(permission.id); } @@ -183,8 +173,7 @@ export function PermissionsPanel() { startAction(HELPER_PERMISSION_ID, "disable"); setNotice(null); try { - const report = await client.darwin.helperDisable(); - setNotice({ tone: "info", message: report.detail }); + await client.darwin.helperDisable(); await client.permissions.refresh(); } catch (error) { console.error("Failed to disable the unattended sync helper:", error); @@ -197,6 +186,24 @@ export function PermissionsPanel() { } } + /** Retry a wanted helper without changing the user's standing decision. */ + async function handleRetryHelper() { + startAction(HELPER_PERMISSION_ID, "retry"); + setNotice(null); + try { + await client.darwin.helperRetry(); + await client.permissions.refresh(); + } catch (error) { + console.error("Failed to retry the unattended sync helper:", error); + setNotice({ + tone: "error", + message: `Retrying the unattended sync helper failed: ${error instanceof Error ? error.message : String(error)}`, + }); + } finally { + finishAction(HELPER_PERMISSION_ID); + } + } + const permissions = permissionsState?.permissions ?? []; return ( @@ -233,10 +240,12 @@ export function PermissionsPanel() { const isGranted = perm.status === "granted"; const pendingAction = requesting.get(perm.id) ?? null; const isGranting = pendingAction === "grant"; + const isRetrying = pendingAction === "retry"; const isDisabling = pendingAction === "disable"; - // The one action this row offers. The helper is the one permission - // nixmac installs rather than asks macOS for, so its row can hand it - // back — a toggle of the standing decision, not of the row's status. + // The helper is the one permission nixmac installs rather than asks + // macOS for, so its normal action toggles the standing decision, not + // the row's status. Retry repeats the stored decision, including a + // failed removal. An enabled helper also keeps Disable available. // While macOS holds the registration pending approval the backend // clears `canRequestProgrammatically` and the row deep-links to Login // Items instead. An action in flight keeps its own button: it has @@ -245,8 +254,23 @@ export function PermissionsPanel() { perm.id === HELPER_PERMISSION_ID && perm.canRequestProgrammatically && (helperPreference === "granted" || isGranted); + const offersRetry = + perm.id === HELPER_PERMISSION_ID && + (isRetrying || + (pendingAction === null && + perm.helperPhase === "failed" && + (helperPreference === "granted" || helperPreference === "disabled"))); const action: "grant" | "disable" | null = - pendingAction ?? (offersDisable ? "disable" : isGranted ? null : "grant"); + pendingAction === "retry" + ? null + : (pendingAction ?? + (offersDisable + ? "disable" + : isGranted || + perm.helperPhase === "needsUserAction" || + (offersRetry && helperPreference === "disabled") + ? null + : "grant")); // A fresh DOM node whenever the button changes shape: the // design-system Button animates a reused one, cross-fading fill and // width between shapes. @@ -331,6 +355,14 @@ export function PermissionsPanel() { Granted ) : null} + {offersRetry ? ( + void handleRetryHelper()} + /> + ) : null} {action === "disable" ? ( = {}): RepairInputs { permissions: makeGrantedPermissions(), helperRow: null, helperPreference: "unset", - helperGraceElapsed: false, skipPermissions: false, nixInstalledOverride: false, ...overrides, @@ -95,7 +94,6 @@ describe("computeRepairPlan", () => { makeInputs({ helperPreference, helperRow: makeHelperRow(), - helperGraceElapsed: true, }), ); expect(plan.banners).toEqual([]); @@ -106,35 +104,59 @@ describe("computeRepairPlan", () => { const plan = computeRepairPlan( makeInputs({ helperPreference: "granted", - helperRow: makeHelperRow({ status: "granted" }), - helperGraceElapsed: true, + helperRow: makeHelperRow({ status: "granted", helperPhase: "ready" }), }), ); expect(plan.banners).toEqual([]); }); - it("waits for the grace period before naming the helper", () => { + it("banners the wanted helper with the backend's typed phase and copy", () => { const plan = computeRepairPlan( makeInputs({ helperPreference: "granted", helperRow: makeHelperRow(), - helperGraceElapsed: false, }), ); - expect(plan.banners).toEqual([]); + expect(plan.banners).toEqual([ + { + kind: "helper-inactive", + phase: "approvalRequired", + instructions: APPROVE_IN_LOGIN_ITEMS, + }, + ]); }); - it("banners the wanted-but-silent helper with the row's own sentence", () => { + it("preserves every actionable helper phase", () => { + for (const phase of [ + "reconciling", + "waitingForActivation", + "needsUserAction", + "failed", + ] as const) { + const plan = computeRepairPlan( + makeInputs({ + helperPreference: "granted", + helperRow: makeHelperRow({ + helperPhase: phase, + canRequestProgrammatically: true, + instructions: `copy for ${phase}`, + }), + }), + ); + expect(plan.banners).toEqual([ + { kind: "helper-inactive", phase, instructions: `copy for ${phase}` }, + ]); + } + }); + + it("does not treat a crossed disabled row as an intentional opt-out", () => { const plan = computeRepairPlan( makeInputs({ helperPreference: "granted", - helperRow: makeHelperRow(), - helperGraceElapsed: true, + helperRow: makeHelperRow({ helperPhase: "disabled" }), }), ); - expect(plan.banners).toEqual([ - { kind: "helper-inactive", instructions: APPROVE_IN_LOGIN_ITEMS }, - ]); + expect(plan.banners[0]).toMatchObject({ kind: "helper-inactive", phase: "failed" }); }); it("leaves the helper out of the revoked-permissions list", () => { @@ -158,12 +180,15 @@ describe("computeRepairPlan", () => { }, helperPreference: "granted", helperRow, - helperGraceElapsed: true, }), ); expect(plan.banners).toEqual([ { kind: "permissions-revoked", missing: [{ id: "full-disk", name: "Full Disk Access" }] }, - { kind: "helper-inactive", instructions: APPROVE_IN_LOGIN_ITEMS }, + { + kind: "helper-inactive", + phase: "approvalRequired", + instructions: APPROVE_IN_LOGIN_ITEMS, + }, ]); }); @@ -178,11 +203,14 @@ describe("computeRepairPlan", () => { }, helperPreference: "granted", helperRow, - helperGraceElapsed: true, }), ); expect(plan.banners).toEqual([ - { kind: "helper-inactive", instructions: APPROVE_IN_LOGIN_ITEMS }, + { + kind: "helper-inactive", + phase: "approvalRequired", + instructions: APPROVE_IN_LOGIN_ITEMS, + }, ]); }); @@ -199,7 +227,6 @@ describe("computeRepairPlan", () => { }, helperPreference: "granted", helperRow: makeHelperRow(), - helperGraceElapsed: true, }), ); expect(plan).toEqual({ blocking: null, banners: [] }); diff --git a/apps/native/src/components/widget/repair/lib.ts b/apps/native/src/components/widget/repair/lib.ts index 19d89ba33..d03358dca 100644 --- a/apps/native/src/components/widget/repair/lib.ts +++ b/apps/native/src/components/widget/repair/lib.ts @@ -6,8 +6,12 @@ export type RepairIssue = | { kind: "config-missing"; configDir: string } | { kind: "nix-missing" } | { kind: "permissions-revoked"; missing: { id: string; name: string }[] } - /** The user asked for the unattended sync helper and it is not answering. */ - | { kind: "helper-inactive"; instructions: string | null }; + /** The user asked for the unattended sync helper and it is not ready. */ + | { + kind: "helper-inactive"; + phase: NonNullable; + instructions: string | null; + }; export interface RepairInputs { /** Onboarding completion latch; repair is a post-completion concept. */ @@ -25,8 +29,6 @@ export interface RepairInputs { helperRow: Permission | null; /** The user's standing decision about the helper; null before hydration. */ helperPreference: HelperPreference | null; - /** Whether the helper condition has held long enough to be worth saying. */ - helperGraceElapsed: boolean; /** Dev-profile overrides; a skipped gate must not resurface as a repair. */ skipPermissions: boolean; nixInstalledOverride: boolean; @@ -46,7 +48,7 @@ export interface RepairPlan { * Only a missing configuration blocks: every main surface operates on it, * while the other regressions degrade specific actions and banner instead. * - * Every input except the three helper ones is snapshotted when the widget + * Every input except the two helper ones is snapshotted when the widget * mounts and re-read only through "Check again", keeping the blocking card * decided once per launch, as D7 requires. The helper's inputs are live: its * row is expected to be wrong for the first seconds of a launch and to come @@ -81,23 +83,27 @@ export function computeRepairPlan(inputs: RepairInputs): RepairPlan { } } - // The user asked for the helper and the helper is not answering. The row - // carries no marker for *why* (approval pending, a misplaced copy, a failed - // register), so one headline states the condition and the row's own - // `instructions` say what is true right now. For the same reason the button - // is the row's Grant action in every state: it opens Login Items when - // approval is pending, and in the states no run can fix it re-reports the - // same sentence — the accepted cost of having nothing to branch on, not a - // gap to close by inventing a marker. + // The helper reverses the old no-marker decision deliberately: approval, + // progress, an active sync, displacement, and failure require different + // copy and actions. The backend owns this phase so every surface sees the + // same state and no UI parses diagnostic prose. if ( !inputs.skipPermissions && inputs.helperPreference === "granted" && inputs.helperRow !== null && - inputs.helperRow.status !== "granted" && - inputs.helperGraceElapsed + inputs.helperRow.status !== "granted" ) { + const observedPhase = + inputs.helperRow.helperPhase ?? + (inputs.helperRow.canRequestProgrammatically ? "reconciling" : "approvalRequired"); + // Defensive only: a correct reconciliation cannot report NoHelper/Removed + // while the stored preference is Granted. If state ever arrives crossed, + // do not silently render it as an intentional opt-out. + const phase = + observedPhase === "disabled" || observedPhase === "ready" ? "failed" : observedPhase; banners.push({ kind: "helper-inactive", + phase, instructions: inputs.helperRow.instructions ?? null, }); } diff --git a/apps/native/src/components/widget/repair/repair.test.tsx b/apps/native/src/components/widget/repair/repair.test.tsx index 3c2f74d66..edfdd9972 100644 --- a/apps/native/src/components/widget/repair/repair.test.tsx +++ b/apps/native/src/components/widget/repair/repair.test.tsx @@ -7,32 +7,40 @@ import { makeNixInstallState, } from "@/utils/test-fixtures"; import { initialViewModelState, useViewModel } from "@nixmac/state"; -import { act, renderHook } from "@testing-library/react"; -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { HELPER_GRACE_MS, useRepair } from "./repair"; +import { act, fireEvent, render, renderHook, screen, waitFor } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { RepairBanners, useRepair } from "./repair"; /** * The helper's banner is the one part of the repair plan that follows live state, - * so what is worth testing is the timing: it must stay quiet while the backend is - * still converging, speak once the condition has outlasted that, and go when the - * helper answers — without a churning report ever holding it back. + * so it must follow the backend's typed phase and clear as soon as the helper + * answers. Retry/approval actions are distinct and tested at the render surface. */ +const { mockHelperRetry, mockPermissionRequest, mockRefresh } = vi.hoisted(() => ({ + mockHelperRetry: vi.fn<() => Promise>(async () => ({})), + mockPermissionRequest: vi.fn< + (input: { permissionId: string }) => Promise + >(async () => ({})), + mockRefresh: vi.fn<() => Promise>(async () => {}), +})); + vi.mock("@/ipc/api", () => ({ tauriAPI: { - permissions: { refresh: vi.fn(async () => {}) }, + permissions: { refresh: mockRefresh }, }, })); vi.mock("@/lib/orpc", () => ({ client: { - flake: { exists: vi.fn(async () => true) }, - permissions: { request: vi.fn(async () => {}) }, + flake: { exists: vi.fn<() => Promise>(async () => true) }, + darwin: { helperRetry: mockHelperRetry }, + permissions: { request: mockPermissionRequest }, }, })); vi.mock("@/lib/env", () => ({ settings: {} })); -vi.mock("@/router", () => ({ nav: { openSettings: vi.fn() } })); +vi.mock("@/router", () => ({ nav: { openSettings: vi.fn<(tab?: string) => void>() } })); vi.mock("@/components/widget/onboarding/restart-setup", () => ({ RestartSetupConfirmation: () => null, })); @@ -67,12 +75,8 @@ function publishHelperRow(helperRow: Permission) { } describe("useRepair", () => { - beforeEach(() => { - vi.useFakeTimers(); - }); - afterEach(() => { - vi.useRealTimers(); + vi.clearAllMocks(); useViewModel.setState(initialViewModelState); }); @@ -83,88 +87,175 @@ describe("useRepair", () => { return view; } - it("stays quiet until the condition has outlasted the grace period", async () => { + it("shows approval immediately", async () => { seedStore(makeHelperRow(), "granted"); const { result } = await mount(); - expect(result.current.plan.banners).toEqual([]); - - await act(async () => { - vi.advanceTimersByTime(HELPER_GRACE_MS); - }); - expect(result.current.plan.banners).toEqual([ - { kind: "helper-inactive", instructions: APPROVE_IN_LOGIN_ITEMS }, + { + kind: "helper-inactive", + phase: "approvalRequired", + instructions: APPROVE_IN_LOGIN_ITEMS, + }, ]); }); - it("does not restart the clock when a still-failing report changes", async () => { - seedStore(makeHelperRow(), "granted"); + it("follows reconciling and active-sync phases without parsing copy", async () => { + seedStore( + makeHelperRow({ + helperPhase: "reconciling", + canRequestProgrammatically: true, + instructions: "nixmac is finishing unattended sync setup.", + }), + "granted", + ); const { result } = await mount(); - await act(async () => { - vi.advanceTimersByTime(HELPER_GRACE_MS * 0.6); + expect(result.current.plan.banners[0]).toMatchObject({ + kind: "helper-inactive", + phase: "reconciling", + }); + + act(() => { publishHelperRow( makeHelperRow({ + helperPhase: "waitingForActivation", canRequestProgrammatically: true, - instructions: "nixmac could not register the unattended sync helper.", + instructions: "Waiting for the current sync.", }), ); }); - expect(result.current.plan.banners).toEqual([]); - - // Past the grace period counted from the first report, not the second. - await act(async () => { - vi.advanceTimersByTime(HELPER_GRACE_MS * 0.6); - }); expect(result.current.plan.banners).toEqual([ { kind: "helper-inactive", - instructions: "nixmac could not register the unattended sync helper.", + phase: "waitingForActivation", + instructions: "Waiting for the current sync.", }, ]); }); - it("clears the banner and the clock once the helper answers", async () => { - seedStore(makeHelperRow(), "granted"); + it("clears the banner once the helper answers", async () => { + seedStore(makeHelperRow({ helperPhase: "failed" }), "granted"); const { result } = await mount(); - - await act(async () => { - vi.advanceTimersByTime(HELPER_GRACE_MS); - }); expect(result.current.plan.banners).toHaveLength(1); - await act(async () => { + act(() => { publishHelperRow( makeHelperRow({ status: "granted", + helperPhase: "ready", canRequestProgrammatically: true, instructions: "The unattended sync helper is installed and answering.", }), ); }); expect(result.current.plan.banners).toEqual([]); + }); - // The clock was cleared with the banner, so a later regression waits again. - await act(async () => { - publishHelperRow(makeHelperRow()); - }); + it("does not let dismissal of one phase hide a later failure", async () => { + seedStore(makeHelperRow(), "granted"); + const { result } = await mount(); + + act(() => result.current.dismissBanner("helper-inactive")); expect(result.current.plan.banners).toEqual([]); - await act(async () => { - vi.advanceTimersByTime(HELPER_GRACE_MS); + act(() => { + publishHelperRow( + makeHelperRow({ + helperPhase: "failed", + canRequestProgrammatically: true, + instructions: "Try again.", + }), + ); + }); + expect(result.current.plan.banners[0]).toMatchObject({ + kind: "helper-inactive", + phase: "failed", }); - expect(result.current.plan.banners).toHaveLength(1); }); it("says nothing about a helper the user did not ask for", async () => { seedStore(makeHelperRow(), "unset"); const { result } = await mount(); - await act(async () => { - vi.advanceTimersByTime(HELPER_GRACE_MS); - }); - expect(result.current.plan.banners).toEqual([]); }); }); + +describe("RepairBanners helper actions", () => { + afterEach(() => vi.clearAllMocks()); + + it("opens System Settings only for approval", async () => { + render( + {}} + onRecheck={mockRefresh} + />, + ); + fireEvent.click(screen.getByRole("button", { name: "Open System Settings" })); + await waitFor(() => expect(mockPermissionRequest).toHaveBeenCalled()); + expect(mockHelperRetry).not.toHaveBeenCalled(); + }); + + it("retries only a failed helper", async () => { + render( + {}} + onRecheck={mockRefresh} + />, + ); + fireEvent.click(screen.getByRole("button", { name: "Retry" })); + await waitFor(() => expect(mockHelperRetry).toHaveBeenCalled()); + expect(mockPermissionRequest).not.toHaveBeenCalled(); + }); + + it("offers no repair action while waiting for an activation", () => { + render( + {}} + onRecheck={mockRefresh} + />, + ); + expect(screen.queryByRole("button", { name: "Retry" })).toBeNull(); + expect(screen.queryByRole("button", { name: "Open System Settings" })).toBeNull(); + }); + + it("keeps move or restart guidance instead of offering an ineffective retry", () => { + render( + {}} + onRecheck={mockRefresh} + />, + ); + expect(screen.getByText("Move nixmac to /Applications and restart it.")).toBeTruthy(); + expect(screen.queryByRole("button", { name: "Retry" })).toBeNull(); + }); +}); diff --git a/apps/native/src/components/widget/repair/repair.tsx b/apps/native/src/components/widget/repair/repair.tsx index 89174b754..49e1060e2 100644 --- a/apps/native/src/components/widget/repair/repair.tsx +++ b/apps/native/src/components/widget/repair/repair.tsx @@ -5,6 +5,7 @@ import { RestartSetupConfirmation } from "@/components/widget/onboarding/restart import { tauriAPI } from "@/ipc/api"; import { settings } from "@/lib/env"; import { client } from "@/lib/orpc"; +import { cn } from "@/lib/utils"; import { nav } from "@/router"; import { useViewModel } from "@nixmac/state"; import { CircleAlert, FolderX, Loader2, RotateCcw, X } from "lucide-react"; @@ -20,17 +21,9 @@ import { /** The inputs read once, when the (hydrated) widget mounts. */ type LaunchSnapshot = Omit< RepairInputs, - "helperRow" | "helperPreference" | "helperGraceElapsed" + "helperRow" | "helperPreference" >; -/** - * How long "the helper was asked for and is not answering" must hold before - * the banner shows. Not a cosmetic debounce: on any launch that re-registers - * the helper the condition is true at mount with the backend already working - * on it, and the grace lets those finish in silence. - */ -export const HELPER_GRACE_MS = 10_000; - /** * Post-completion prerequisite regressions: what to block on, what to banner. * @@ -57,21 +50,11 @@ export function useRepair(): { ); const helperPreference = useViewModel((s) => s.preferences?.helperPreference ?? null); - // The grace clock keys on this boolean, not on the row: every publish is a - // fresh object, and a helper whose registration keeps failing publishes one - // per pass — restarting the clock each time would keep the banner off the - // screen for exactly as long as the failure lasts. - const helperWanted = - helperPreference === "granted" && helperRow !== null && helperRow.status !== "granted"; - const [helperGraceElapsed, setHelperGraceElapsed] = useState(false); + // Dismissal belongs to the state the user saw. A later failure must not stay + // hidden because the user dismissed an earlier progress or approval notice. useEffect(() => { - if (!helperWanted) { - setHelperGraceElapsed(false); - return; - } - const timer = setTimeout(() => setHelperGraceElapsed(true), HELPER_GRACE_MS); - return () => clearTimeout(timer); - }, [helperWanted]); + setDismissed((current) => current.filter((kind) => kind !== "helper-inactive")); + }, [helperRow?.helperPhase]); const evaluate = useCallback(async () => { // Snapshot the store rather than subscribing: everything here is @@ -114,7 +97,7 @@ export function useRepair(): { }, [evaluate]); const plan = snapshot - ? computeRepairPlan({ ...snapshot, helperRow, helperPreference, helperGraceElapsed }) + ? computeRepairPlan({ ...snapshot, helperRow, helperPreference }) : { blocking: null, banners: [] }; return { @@ -128,36 +111,46 @@ export function useRepair(): { } /** - * The same Grant the helper's row in Settings → Permissions offers; it may - * open Login Items when macOS is waiting for approval there. No re-probe - * afterwards: this banner reads the row live, published on every - * reconciliation pass, so a failed call is logged rather than surfaced. The - * label keeps its box with the spinner laid over it so the running button - * never resizes (the design-system Button animates resizes). + * The helper's two repair actions are intentionally distinct: approval records + * the opt-in and may open Login Items, while Retry preserves the standing + * decision and only runs reconciliation again. */ -function HelperGrantButton() { - const [granting, setGranting] = useState(false); +function HelperActionButton({ + action, + onRecheck, +}: { + action: "approval" | "retry"; + onRecheck: () => Promise; +}) { + const [working, setWorking] = useState(false); + const idle = action === "approval" ? "Open System Settings" : "Retry"; + const busy = action === "approval" ? "Opening…" : "Retrying…"; return ( )} - {issue.kind === "helper-inactive" && } + {issue.kind === "helper-inactive" && issue.phase === "approvalRequired" ? ( + + ) : null} + {issue.kind === "helper-inactive" && issue.phase === "failed" ? ( + + ) : null}
Streaming evolve
Stream the model's response into the evolve progress view as it is generated, instead of waiting for each full reply. Applies to the next evolution.
Feature flags

Override PostHog feature flag variants locally for testing. "PostHog default" uses the server-side value; choosing a variant forces it even when diagnostics are disabled.

Update channel

Stable follows releases from main. Develop follows signed release-mode builds from develop. Version pins override the selected channel until you resume auto-update.

Current channel: develop
Version pin
Pinned to v0.21.0. The silent update check on launch is suppressed while pinned.
Install a past release

Enter a version that exists in the release bucket (look at git tag for valid values). The signed bundle is downloaded from releases.nixmac.com/<version>/, verified, and installed.

State reset

Reset saved Tauri plugin-store data when the widget gets stuck in the wrong step or cached data looks stale. This clears saved settings, routing state, build state, prompt history, and model caches.

"`; +exports[`Pinned To Past Version 1`] = `"

Developer

Hidden tools for debugging and bisecting regressions. Don't use these unless you know what you're doing.

Heads up
Installing a past version replaces your current .app bundle on disk. The version number you enter must match an existing release at releases.nixmac.com. Bisecting only works in release builds — the dev binary doesn't ship the updater plugin.
Experimental
Spinning mascot
Show a spinning 3D nixmac mascot in the bottom-right corner of your screen while an evolution or build is running. Takes effect after restarting nixmac.
Streaming evolve
Stream the model's response into the evolve progress view as it is generated, instead of waiting for each full reply. Applies to the next evolution.
Feature flags

Override feature flag variants locally for testing. "PostHog default" uses the server-side value for PostHog-backed flags; local-only flags name their app default. Choosing a variant stores a local override even when diagnostics are disabled.

Launch-time local override; this flag is not read from PostHog. Takes effect after restart.

Update channel

Stable follows releases from main. Develop follows signed release-mode builds from develop. Version pins override the selected channel until you resume auto-update.

Current channel: develop
Version pin
Pinned to v0.21.0. The silent update check on launch is suppressed while pinned.
Install a past release

Enter a version that exists in the release bucket (look at git tag for valid values). The signed bundle is downloaded from releases.nixmac.com/<version>/, verified, and installed.

State reset

Reset saved Tauri plugin-store data when the widget gets stuck in the wrong step or cached data looks stale. This clears saved settings, routing state, build state, prompt history, and model caches.

"`; -exports[`Unpinned 1`] = `"

Developer

Hidden tools for debugging and bisecting regressions. Don't use these unless you know what you're doing.

Heads up
Installing a past version replaces your current .app bundle on disk. The version number you enter must match an existing release at releases.nixmac.com. Bisecting only works in release builds — the dev binary doesn't ship the updater plugin.
Experimental
Spinning mascot
Show a spinning 3D nixmac mascot in the bottom-right corner of your screen while an evolution or build is running. Takes effect after restarting nixmac.
Streaming evolve
Stream the model's response into the evolve progress view as it is generated, instead of waiting for each full reply. Applies to the next evolution.
Feature flags

Override PostHog feature flag variants locally for testing. "PostHog default" uses the server-side value; choosing a variant forces it even when diagnostics are disabled.

Update channel

Stable follows releases from main. Develop follows signed release-mode builds from develop. Version pins override the selected channel until you resume auto-update.

Current channel: stable
Version pin
Not pinned. Currently running v0.22.0.
Install a past release

Enter a version that exists in the release bucket (look at git tag for valid values). The signed bundle is downloaded from releases.nixmac.com/<version>/, verified, and installed.

State reset

Reset saved Tauri plugin-store data when the widget gets stuck in the wrong step or cached data looks stale. This clears saved settings, routing state, build state, prompt history, and model caches.

"`; +exports[`Unpinned 1`] = `"

Developer

Hidden tools for debugging and bisecting regressions. Don't use these unless you know what you're doing.

Heads up
Installing a past version replaces your current .app bundle on disk. The version number you enter must match an existing release at releases.nixmac.com. Bisecting only works in release builds — the dev binary doesn't ship the updater plugin.
Experimental
Spinning mascot
Show a spinning 3D nixmac mascot in the bottom-right corner of your screen while an evolution or build is running. Takes effect after restarting nixmac.
Streaming evolve
Stream the model's response into the evolve progress view as it is generated, instead of waiting for each full reply. Applies to the next evolution.
Feature flags

Override feature flag variants locally for testing. "PostHog default" uses the server-side value for PostHog-backed flags; local-only flags name their app default. Choosing a variant stores a local override even when diagnostics are disabled.

Launch-time local override; this flag is not read from PostHog. Takes effect after restart.

Update channel

Stable follows releases from main. Develop follows signed release-mode builds from develop. Version pins override the selected channel until you resume auto-update.

Current channel: stable
Version pin
Not pinned. Currently running v0.22.0.
Install a past release

Enter a version that exists in the release bucket (look at git tag for valid values). The signed bundle is downloaded from releases.nixmac.com/<version>/, verified, and installed.

State reset

Reset saved Tauri plugin-store data when the widget gets stuck in the wrong step or cached data looks stale. This clears saved settings, routing state, build state, prompt history, and model caches.

"`; diff --git a/apps/native/src/components/widget/settings/developer-tab.tsx b/apps/native/src/components/widget/settings/developer-tab.tsx index e73b9e442..22d0f69b9 100644 --- a/apps/native/src/components/widget/settings/developer-tab.tsx +++ b/apps/native/src/components/widget/settings/developer-tab.tsx @@ -243,15 +243,19 @@ export function DeveloperTab() {

- Override PostHog feature flag variants locally for testing. "PostHog - default" uses the server-side value; choosing a variant forces it - even when diagnostics are disabled. + Override feature flag variants locally for testing. "PostHog default" + uses the server-side value for PostHog-backed flags; local-only flags + name their app default. Choosing a variant stores a local override even + when diagnostics are disabled.

{OVERRIDABLE_FLAGS.map((flag) => { const active = featureFlagOverrides?.[flag.key] ?? null; return (
+ {flag.description ? ( +

{flag.description}

+ ) : null}
{flag.options.map((option) => (