From c66336ccec272633a8ddb73654d04baac2f4289a Mon Sep 17 00:00:00 2001 From: Rasmus Widing Date: Sat, 28 Feb 2026 10:18:44 +0200 Subject: [PATCH 1/2] refactor(notify): extract NotificationBackend trait with platform backends Add trait-based extensibility for the notification subsystem, following the existing TerminalBackend and ForgeBackend patterns: - NotificationBackend trait with name/is_available/send interface - MacOsNotificationBackend (osascript) and LinuxNotificationBackend (notify-send) - Registry with platform-based auto-detection - NotifyError type with KildError implementation Also add standard From/TryFrom trait implementations: - From for OpenMode (kild-protocol) - From for core SessionStatus - From<&ProcessInfo> for ProcessMetadata Closes #435 --- crates/kild-core/src/lib.rs | 1 + crates/kild-core/src/notify/backends/linux.rs | 62 +++++++++++ crates/kild-core/src/notify/backends/macos.rs | 61 ++++++++++ crates/kild-core/src/notify/backends/mod.rs | 7 ++ crates/kild-core/src/notify/errors.rs | 73 ++++++++++++ crates/kild-core/src/notify/mod.rs | 104 +++--------------- crates/kild-core/src/notify/registry.rs | 86 +++++++++++++++ crates/kild-core/src/notify/traits.rs | 66 +++++++++++ crates/kild-core/src/process/types.rs | 66 +++++++++++ crates/kild-core/src/sessions/types/status.rs | 19 ++++ crates/kild-core/src/sessions/types/tests.rs | 20 ++++ crates/kild-protocol/src/types.rs | 28 +++++ 12 files changed, 503 insertions(+), 90 deletions(-) create mode 100644 crates/kild-core/src/notify/backends/linux.rs create mode 100644 crates/kild-core/src/notify/backends/macos.rs create mode 100644 crates/kild-core/src/notify/backends/mod.rs create mode 100644 crates/kild-core/src/notify/errors.rs create mode 100644 crates/kild-core/src/notify/registry.rs create mode 100644 crates/kild-core/src/notify/traits.rs diff --git a/crates/kild-core/src/lib.rs b/crates/kild-core/src/lib.rs index 1727a0b2..d49609a2 100644 --- a/crates/kild-core/src/lib.rs +++ b/crates/kild-core/src/lib.rs @@ -51,6 +51,7 @@ pub use kild_config::{ Keybindings, KildConfig, TerminalConfig, UiConfig, VALID_TERMINALS, }; pub use kild_config::{CopyOptions, IncludeConfig, PatternRule}; +pub use notify::{NotificationBackend, NotifyError}; pub use projects::{Project, ProjectError, ProjectManager, ProjectsData}; pub use sessions::agent_status::AgentStatusResult; pub use sessions::info::SessionInfo; diff --git a/crates/kild-core/src/notify/backends/linux.rs b/crates/kild-core/src/notify/backends/linux.rs new file mode 100644 index 00000000..bd89535e --- /dev/null +++ b/crates/kild-core/src/notify/backends/linux.rs @@ -0,0 +1,62 @@ +//! Linux notification backend using notify-send (libnotify). + +use crate::notify::errors::NotifyError; +use crate::notify::traits::NotificationBackend; + +/// Linux notification backend via `notify-send` (libnotify). +pub struct LinuxNotificationBackend; + +impl NotificationBackend for LinuxNotificationBackend { + fn name(&self) -> &'static str { + "linux" + } + + fn is_available(&self) -> bool { + cfg!(target_os = "linux") && which::which("notify-send").is_ok() + } + + fn send(&self, title: &str, message: &str) -> Result<(), NotifyError> { + if which::which("notify-send").is_err() { + return Err(NotifyError::ToolNotFound { + tool: "notify-send".to_string(), + }); + } + + let output = std::process::Command::new("notify-send") + .arg(title) + .arg(message) + .output() + .map_err(|e| NotifyError::SendFailed { + message: format!("notify-send exec failed: {}", e), + })?; + + if output.status.success() { + Ok(()) + } else { + let stderr = String::from_utf8_lossy(&output.stderr); + Err(NotifyError::SendFailed { + message: format!("notify-send exit {}: {}", output.status, stderr.trim()), + }) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn linux_backend_name() { + let backend = LinuxNotificationBackend; + assert_eq!(backend.name(), "linux"); + } + + #[test] + fn linux_backend_availability_matches_platform() { + let backend = LinuxNotificationBackend; + // On non-Linux platforms, should always be unavailable + if !cfg!(target_os = "linux") { + assert!(!backend.is_available()); + } + } +} diff --git a/crates/kild-core/src/notify/backends/macos.rs b/crates/kild-core/src/notify/backends/macos.rs new file mode 100644 index 00000000..113ad73f --- /dev/null +++ b/crates/kild-core/src/notify/backends/macos.rs @@ -0,0 +1,61 @@ +//! macOS notification backend using osascript (Notification Center). + +use crate::escape::applescript_escape; +use crate::notify::errors::NotifyError; +use crate::notify::traits::NotificationBackend; + +/// macOS notification backend via `osascript` (Notification Center). +pub struct MacOsNotificationBackend; + +impl NotificationBackend for MacOsNotificationBackend { + fn name(&self) -> &'static str { + "macos" + } + + fn is_available(&self) -> bool { + cfg!(target_os = "macos") + } + + fn send(&self, title: &str, message: &str) -> Result<(), NotifyError> { + let escaped_title = applescript_escape(title); + let escaped_message = applescript_escape(message); + let script = format!( + r#"display notification "{}" with title "{}""#, + escaped_message, escaped_title + ); + + let output = std::process::Command::new("osascript") + .arg("-e") + .arg(&script) + .output() + .map_err(|e| NotifyError::SendFailed { + message: format!("osascript exec failed: {}", e), + })?; + + if output.status.success() { + Ok(()) + } else { + let stderr = String::from_utf8_lossy(&output.stderr); + Err(NotifyError::SendFailed { + message: format!("osascript exit {}: {}", output.status, stderr.trim()), + }) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn macos_backend_name() { + let backend = MacOsNotificationBackend; + assert_eq!(backend.name(), "macos"); + } + + #[test] + fn macos_backend_availability_matches_platform() { + let backend = MacOsNotificationBackend; + assert_eq!(backend.is_available(), cfg!(target_os = "macos")); + } +} diff --git a/crates/kild-core/src/notify/backends/mod.rs b/crates/kild-core/src/notify/backends/mod.rs new file mode 100644 index 00000000..7f91461f --- /dev/null +++ b/crates/kild-core/src/notify/backends/mod.rs @@ -0,0 +1,7 @@ +//! Notification backend implementations. + +mod linux; +mod macos; + +pub use linux::LinuxNotificationBackend; +pub use macos::MacOsNotificationBackend; diff --git a/crates/kild-core/src/notify/errors.rs b/crates/kild-core/src/notify/errors.rs new file mode 100644 index 00000000..b8cef141 --- /dev/null +++ b/crates/kild-core/src/notify/errors.rs @@ -0,0 +1,73 @@ +//! Notification error types. + +use crate::errors::KildError; + +#[derive(Debug, thiserror::Error)] +pub enum NotifyError { + #[error("Notification tool not found: {tool}")] + ToolNotFound { tool: String }, + + #[error("Notification failed: {message}")] + SendFailed { message: String }, + + #[error("IO error during notification: {source}")] + IoError { + #[from] + source: std::io::Error, + }, +} + +impl KildError for NotifyError { + fn error_code(&self) -> &'static str { + match self { + NotifyError::ToolNotFound { .. } => "NOTIFY_TOOL_NOT_FOUND", + NotifyError::SendFailed { .. } => "NOTIFY_SEND_FAILED", + NotifyError::IoError { .. } => "NOTIFY_IO_ERROR", + } + } + + fn is_user_error(&self) -> bool { + matches!(self, NotifyError::ToolNotFound { .. }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_tool_not_found() { + let error = NotifyError::ToolNotFound { + tool: "notify-send".to_string(), + }; + assert_eq!( + error.to_string(), + "Notification tool not found: notify-send" + ); + assert_eq!(error.error_code(), "NOTIFY_TOOL_NOT_FOUND"); + assert!(error.is_user_error()); + } + + #[test] + fn test_send_failed() { + let error = NotifyError::SendFailed { + message: "osascript exited with code 1".to_string(), + }; + assert_eq!( + error.to_string(), + "Notification failed: osascript exited with code 1" + ); + assert_eq!(error.error_code(), "NOTIFY_SEND_FAILED"); + assert!(!error.is_user_error()); + } + + #[test] + fn test_io_error() { + let error = NotifyError::IoError { + source: std::io::Error::new(std::io::ErrorKind::NotFound, "file not found"), + }; + assert!(error.to_string().contains("IO error")); + assert_eq!(error.error_code(), "NOTIFY_IO_ERROR"); + assert!(!error.is_user_error()); + } +} diff --git a/crates/kild-core/src/notify/mod.rs b/crates/kild-core/src/notify/mod.rs index de02702d..d1e8c91b 100644 --- a/crates/kild-core/src/notify/mod.rs +++ b/crates/kild-core/src/notify/mod.rs @@ -3,13 +3,21 @@ //! Best-effort notifications — failures are logged but never propagate. //! Used by `kild agent-status --notify` to alert when an agent enters //! `Waiting` or `Error` status. +//! +//! Notifications are dispatched via the [`NotificationBackend`] trait, +//! with platform-specific backends registered in [`registry`]. + +pub mod backends; +pub mod errors; +pub mod registry; +pub mod traits; + +pub use errors::NotifyError; +pub use traits::NotificationBackend; use kild_protocol::AgentStatus; use tracing::{info, warn}; -#[cfg(not(target_os = "macos"))] -use tracing::debug; - /// Returns `true` if a notification should be sent for the given status. /// /// Only `Waiting` and `Error` require user attention. @@ -24,10 +32,7 @@ pub fn format_notification_message(agent: &str, branch: &str, status: AgentStatu /// Send a platform-native desktop notification (best-effort). /// -/// - macOS: `osascript` (Notification Center) -/// - Linux: `notify-send` (requires libnotify) -/// - Other: no-op -/// +/// Dispatches to the first available [`NotificationBackend`] via the registry. /// Failures are logged at warn level but never returned as errors. pub fn send_notification(title: &str, message: &str) { info!( @@ -36,83 +41,10 @@ pub fn send_notification(title: &str, message: &str) { message = message, ); - send_platform_notification(title, message); -} - -#[cfg(target_os = "macos")] -fn send_platform_notification(title: &str, message: &str) { - use crate::escape::applescript_escape; - - let escaped_title = applescript_escape(title); - let escaped_message = applescript_escape(message); - let script = format!( - r#"display notification "{}" with title "{}""#, - escaped_message, escaped_title - ); - - match std::process::Command::new("osascript") - .arg("-e") - .arg(&script) - .output() - { - Ok(output) if output.status.success() => { - info!(event = "core.notify.send_completed", title = title); - } - Ok(output) => { - let stderr = String::from_utf8_lossy(&output.stderr); - warn!( - event = "core.notify.send_failed", - title = title, - stderr = %stderr, - ); - } - Err(e) => { - warn!( - event = "core.notify.send_failed", - title = title, - error = %e, - ); - } - } -} - -#[cfg(target_os = "linux")] -fn send_platform_notification(title: &str, message: &str) { - match which::which("notify-send") { - Ok(_) => {} - Err(which::Error::CannotFindBinaryPath) => { - debug!( - event = "core.notify.send_skipped", - reason = "notify-send not found", - ); - return; - } - Err(e) => { - warn!( - event = "core.notify.send_failed", - title = title, - error = %e, - ); - return; - } - } - - match std::process::Command::new("notify-send") - .arg(title) - .arg(message) - .output() - { - Ok(output) if output.status.success() => { + match registry::send_via_backend(title, message) { + Ok(()) => { info!(event = "core.notify.send_completed", title = title); } - Ok(output) => { - let stderr = String::from_utf8_lossy(&output.stderr); - warn!( - event = "core.notify.send_failed", - title = title, - stderr = %stderr, - ); - } Err(e) => { warn!( event = "core.notify.send_failed", @@ -123,14 +55,6 @@ fn send_platform_notification(title: &str, message: &str) { } } -#[cfg(not(any(target_os = "macos", target_os = "linux")))] -fn send_platform_notification(_title: &str, _message: &str) { - debug!( - event = "core.notify.send_skipped", - reason = "unsupported platform", - ); -} - #[cfg(test)] mod tests { use super::*; diff --git a/crates/kild-core/src/notify/registry.rs b/crates/kild-core/src/notify/registry.rs new file mode 100644 index 00000000..134bc1d9 --- /dev/null +++ b/crates/kild-core/src/notify/registry.rs @@ -0,0 +1,86 @@ +//! Notification registry for managing and looking up notification backends. + +use std::sync::LazyLock; + +use tracing::debug; + +use super::backends::{LinuxNotificationBackend, MacOsNotificationBackend}; +use super::traits::NotificationBackend; + +/// Global registry of all supported notification backends. +static REGISTRY: LazyLock = LazyLock::new(NotificationRegistry::new); + +/// Registry that manages all notification backend implementations. +struct NotificationRegistry { + backends: Vec>, +} + +impl NotificationRegistry { + fn new() -> Self { + Self { + backends: vec![ + Box::new(MacOsNotificationBackend), + Box::new(LinuxNotificationBackend), + ], + } + } + + /// Detect the first available notification backend. + fn detect(&self) -> Option<&dyn NotificationBackend> { + self.backends.iter().find_map(|b| { + if b.is_available() { + Some(b.as_ref()) + } else { + None + } + }) + } +} + +/// Detect the first available notification backend for the current platform. +/// +/// Returns `None` on unsupported platforms or when no notification tools are installed. +pub fn detect_backend() -> Option<&'static dyn NotificationBackend> { + REGISTRY.detect() +} + +/// Send a notification via the first available platform backend. +/// +/// Best-effort: returns the backend's `Result` to let callers decide how +/// to handle failures. Returns `Ok(())` with a debug log if no backend is available. +pub fn send_via_backend(title: &str, message: &str) -> Result<(), super::errors::NotifyError> { + let Some(backend) = detect_backend() else { + debug!( + event = "core.notify.send_skipped", + reason = "no backend available", + ); + return Ok(()); + }; + + backend.send(title, message) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn detect_backend_does_not_panic() { + // Should never panic regardless of platform + let _result = detect_backend(); + } + + #[test] + fn registry_contains_expected_backends() { + let registry = NotificationRegistry::new(); + let names: Vec<&str> = registry.backends.iter().map(|b| b.name()).collect(); + assert!(names.contains(&"macos")); + assert!(names.contains(&"linux")); + } + + #[test] + fn send_via_backend_does_not_panic() { + // Best-effort: should never panic even if no backend available + let _result = send_via_backend("Test", "Hello"); + } +} diff --git a/crates/kild-core/src/notify/traits.rs b/crates/kild-core/src/notify/traits.rs new file mode 100644 index 00000000..7c5c698c --- /dev/null +++ b/crates/kild-core/src/notify/traits.rs @@ -0,0 +1,66 @@ +//! Notification backend trait definition. + +use crate::notify::errors::NotifyError; + +/// Trait defining the interface for notification backends. +/// +/// Each supported platform (macOS, Linux) implements this trait +/// to provide platform-specific desktop notification delivery. +pub trait NotificationBackend: Send + Sync { + /// The canonical name of this backend (e.g., "macos", "linux"). + fn name(&self) -> &'static str; + + /// Check if this notification backend is available on the system. + fn is_available(&self) -> bool; + + /// Send a desktop notification. + /// + /// # Arguments + /// * `title` - The notification title + /// * `message` - The notification body text + fn send(&self, title: &str, message: &str) -> Result<(), NotifyError>; +} + +#[cfg(test)] +mod tests { + use super::*; + + struct MockBackend { + available: bool, + } + + impl NotificationBackend for MockBackend { + fn name(&self) -> &'static str { + "mock" + } + + fn is_available(&self) -> bool { + self.available + } + + fn send(&self, _title: &str, _message: &str) -> Result<(), NotifyError> { + if self.available { + Ok(()) + } else { + Err(NotifyError::ToolNotFound { + tool: "mock".to_string(), + }) + } + } + } + + #[test] + fn mock_notification_backend_available() { + let backend = MockBackend { available: true }; + assert_eq!(backend.name(), "mock"); + assert!(backend.is_available()); + assert!(backend.send("Test", "Hello").is_ok()); + } + + #[test] + fn mock_notification_backend_unavailable() { + let backend = MockBackend { available: false }; + assert!(!backend.is_available()); + assert!(backend.send("Test", "Hello").is_err()); + } +} diff --git a/crates/kild-core/src/process/types.rs b/crates/kild-core/src/process/types.rs index d331e120..855f121f 100644 --- a/crates/kild-core/src/process/types.rs +++ b/crates/kild-core/src/process/types.rs @@ -71,6 +71,15 @@ pub struct ProcessMetadata { pub start_time: u64, } +impl From<&ProcessInfo> for ProcessMetadata { + fn from(info: &ProcessInfo) -> Self { + Self { + name: info.name.clone(), + start_time: info.start_time, + } + } +} + #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ProcessMetrics { pub cpu_usage_percent: f32, @@ -82,3 +91,60 @@ impl ProcessMetrics { self.memory_usage_bytes / 1_024 / 1_024 } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_process_metadata_from_process_info() { + let info = ProcessInfo { + pid: Pid::from_raw(1234), + name: "claude".to_string(), + status: ProcessStatus::Running, + start_time: 1700000000, + }; + let metadata: ProcessMetadata = ProcessMetadata::from(&info); + assert_eq!(metadata.name, "claude"); + assert_eq!(metadata.start_time, 1700000000); + } + + #[test] + fn test_process_metadata_from_ref_preserves_original() { + let info = ProcessInfo { + pid: Pid::from_raw(5678), + name: "kiro".to_string(), + status: ProcessStatus::Sleeping, + start_time: 1700001000, + }; + let _metadata: ProcessMetadata = (&info).into(); + // Original info still accessible + assert_eq!(info.name, "kiro"); + assert_eq!(info.pid.as_u32(), 5678); + } + + #[test] + fn test_pid_from_u32() { + let pid: Pid = 42u32.into(); + assert_eq!(pid.as_u32(), 42); + } + + #[test] + fn test_pid_new_rejects_zero() { + assert!(Pid::new(0).is_err()); + } + + #[test] + fn test_pid_new_accepts_nonzero() { + assert!(Pid::new(1).is_ok()); + } + + #[test] + fn test_process_metrics_memory_mb() { + let metrics = ProcessMetrics { + cpu_usage_percent: 10.0, + memory_usage_bytes: 1_024 * 1_024 * 256, + }; + assert_eq!(metrics.memory_usage_mb(), 256); + } +} diff --git a/crates/kild-core/src/sessions/types/status.rs b/crates/kild-core/src/sessions/types/status.rs index 9e5c40af..fefbd962 100644 --- a/crates/kild-core/src/sessions/types/status.rs +++ b/crates/kild-core/src/sessions/types/status.rs @@ -12,6 +12,25 @@ pub enum SessionStatus { Destroyed, } +/// Convert a daemon protocol `SessionStatus` into a core `SessionStatus`. +/// +/// `Running` and `Creating` both map to `Active` (the session is alive). +/// `Stopped` maps to `Stopped`. There is no protocol equivalent for `Destroyed` +/// since destruction is a core-only concept. +impl From for SessionStatus { + fn from(status: kild_protocol::SessionStatus) -> Self { + match status { + kild_protocol::SessionStatus::Running | kild_protocol::SessionStatus::Creating => { + SessionStatus::Active + } + kild_protocol::SessionStatus::Stopped => SessionStatus::Stopped, + // SessionStatus is #[non_exhaustive]; treat unknown variants as Active + // (alive until proven otherwise). + _ => SessionStatus::Active, + } + } +} + impl std::fmt::Display for SessionStatus { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { diff --git a/crates/kild-core/src/sessions/types/tests.rs b/crates/kild-core/src/sessions/types/tests.rs index bb98a97b..0e069e19 100644 --- a/crates/kild-core/src/sessions/types/tests.rs +++ b/crates/kild-core/src/sessions/types/tests.rs @@ -1042,6 +1042,26 @@ fn test_process_status_roundtrip() { } } +// --- From tests --- + +#[test] +fn test_protocol_running_maps_to_active() { + let core_status: SessionStatus = kild_protocol::SessionStatus::Running.into(); + assert_eq!(core_status, SessionStatus::Active); +} + +#[test] +fn test_protocol_creating_maps_to_active() { + let core_status: SessionStatus = kild_protocol::SessionStatus::Creating.into(); + assert_eq!(core_status, SessionStatus::Active); +} + +#[test] +fn test_protocol_stopped_maps_to_stopped() { + let core_status: SessionStatus = kild_protocol::SessionStatus::Stopped.into(); + assert_eq!(core_status, SessionStatus::Stopped); +} + #[test] fn test_session_new_sets_all_fields() { use kild_protocol::RuntimeMode; diff --git a/crates/kild-protocol/src/types.rs b/crates/kild-protocol/src/types.rs index e99d633f..5dd6ebef 100644 --- a/crates/kild-protocol/src/types.rs +++ b/crates/kild-protocol/src/types.rs @@ -237,6 +237,16 @@ pub enum AgentMode { BareShell, } +impl From for OpenMode { + fn from(mode: AgentMode) -> Self { + match mode { + AgentMode::DefaultAgent => OpenMode::DefaultAgent, + AgentMode::Agent(name) => OpenMode::Agent(name), + AgentMode::BareShell => OpenMode::BareShell, + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -528,6 +538,24 @@ mod tests { } } + #[test] + fn test_agent_mode_into_open_mode_default() { + let open: OpenMode = AgentMode::DefaultAgent.into(); + assert_eq!(open, OpenMode::DefaultAgent); + } + + #[test] + fn test_agent_mode_into_open_mode_agent() { + let open: OpenMode = AgentMode::Agent("claude".to_string()).into(); + assert_eq!(open, OpenMode::Agent("claude".to_string())); + } + + #[test] + fn test_agent_mode_into_open_mode_bare_shell() { + let open: OpenMode = AgentMode::BareShell.into(); + assert_eq!(open, OpenMode::BareShell); + } + #[test] fn test_runtime_mode_serializes_as_snake_case() { assert_eq!( From 5618386fb8a0f484a4c030b7522600aa2588042f Mon Sep 17 00:00:00 2001 From: Rasmus Widing Date: Sat, 28 Feb 2026 10:25:39 +0200 Subject: [PATCH 2/2] fix(notify): address review findings from 4 review agents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove dead NotifyError::IoError variant (YAGNI — no caller produces it) - Remove redundant which::which check in LinuxNotificationBackend::send (is_available already gates dispatch; Command::new handles missing binary) - Return bool from send_via_backend to distinguish sent vs skipped, fixing misleading send_completed log when no backend is available - Remove premature lib.rs re-exports of NotificationBackend/NotifyError (no callers outside kild-core) - Simplify registry detect() with find+map instead of verbose find_map - Inline detect_backend() into send_via_backend (single caller) - Add warn log on wildcard arm in From for unknown variants - Add doc note on format_notification_message re "needs input" for Error - Remove redundant test comments --- crates/kild-core/src/lib.rs | 1 - crates/kild-core/src/notify/backends/linux.rs | 7 ---- crates/kild-core/src/notify/errors.rs | 17 -------- crates/kild-core/src/notify/mod.rs | 9 +++- crates/kild-core/src/notify/registry.rs | 42 +++++++------------ crates/kild-core/src/process/types.rs | 1 - crates/kild-core/src/sessions/types/status.rs | 9 +++- 7 files changed, 31 insertions(+), 55 deletions(-) diff --git a/crates/kild-core/src/lib.rs b/crates/kild-core/src/lib.rs index d49609a2..1727a0b2 100644 --- a/crates/kild-core/src/lib.rs +++ b/crates/kild-core/src/lib.rs @@ -51,7 +51,6 @@ pub use kild_config::{ Keybindings, KildConfig, TerminalConfig, UiConfig, VALID_TERMINALS, }; pub use kild_config::{CopyOptions, IncludeConfig, PatternRule}; -pub use notify::{NotificationBackend, NotifyError}; pub use projects::{Project, ProjectError, ProjectManager, ProjectsData}; pub use sessions::agent_status::AgentStatusResult; pub use sessions::info::SessionInfo; diff --git a/crates/kild-core/src/notify/backends/linux.rs b/crates/kild-core/src/notify/backends/linux.rs index bd89535e..b97ee258 100644 --- a/crates/kild-core/src/notify/backends/linux.rs +++ b/crates/kild-core/src/notify/backends/linux.rs @@ -16,12 +16,6 @@ impl NotificationBackend for LinuxNotificationBackend { } fn send(&self, title: &str, message: &str) -> Result<(), NotifyError> { - if which::which("notify-send").is_err() { - return Err(NotifyError::ToolNotFound { - tool: "notify-send".to_string(), - }); - } - let output = std::process::Command::new("notify-send") .arg(title) .arg(message) @@ -54,7 +48,6 @@ mod tests { #[test] fn linux_backend_availability_matches_platform() { let backend = LinuxNotificationBackend; - // On non-Linux platforms, should always be unavailable if !cfg!(target_os = "linux") { assert!(!backend.is_available()); } diff --git a/crates/kild-core/src/notify/errors.rs b/crates/kild-core/src/notify/errors.rs index b8cef141..b57fa303 100644 --- a/crates/kild-core/src/notify/errors.rs +++ b/crates/kild-core/src/notify/errors.rs @@ -9,12 +9,6 @@ pub enum NotifyError { #[error("Notification failed: {message}")] SendFailed { message: String }, - - #[error("IO error during notification: {source}")] - IoError { - #[from] - source: std::io::Error, - }, } impl KildError for NotifyError { @@ -22,7 +16,6 @@ impl KildError for NotifyError { match self { NotifyError::ToolNotFound { .. } => "NOTIFY_TOOL_NOT_FOUND", NotifyError::SendFailed { .. } => "NOTIFY_SEND_FAILED", - NotifyError::IoError { .. } => "NOTIFY_IO_ERROR", } } @@ -60,14 +53,4 @@ mod tests { assert_eq!(error.error_code(), "NOTIFY_SEND_FAILED"); assert!(!error.is_user_error()); } - - #[test] - fn test_io_error() { - let error = NotifyError::IoError { - source: std::io::Error::new(std::io::ErrorKind::NotFound, "file not found"), - }; - assert!(error.to_string().contains("IO error")); - assert_eq!(error.error_code(), "NOTIFY_IO_ERROR"); - assert!(!error.is_user_error()); - } } diff --git a/crates/kild-core/src/notify/mod.rs b/crates/kild-core/src/notify/mod.rs index d1e8c91b..6dd454ed 100644 --- a/crates/kild-core/src/notify/mod.rs +++ b/crates/kild-core/src/notify/mod.rs @@ -26,6 +26,10 @@ pub fn should_notify(notify: bool, status: AgentStatus) -> bool { } /// Format the notification message for an agent status change. +/// +/// The message body always reads "needs input" regardless of status. +/// This covers both `Waiting` (literal input required) and `Error` +/// (user must inspect and unblock the agent). pub fn format_notification_message(agent: &str, branch: &str, status: AgentStatus) -> String { format!("Agent {} in {} needs input ({})", agent, branch, status) } @@ -42,9 +46,12 @@ pub fn send_notification(title: &str, message: &str) { ); match registry::send_via_backend(title, message) { - Ok(()) => { + Ok(true) => { info!(event = "core.notify.send_completed", title = title); } + Ok(false) => { + // No backend available — already logged at debug in registry + } Err(e) => { warn!( event = "core.notify.send_failed", diff --git a/crates/kild-core/src/notify/registry.rs b/crates/kild-core/src/notify/registry.rs index 134bc1d9..b747b78c 100644 --- a/crates/kild-core/src/notify/registry.rs +++ b/crates/kild-core/src/notify/registry.rs @@ -26,50 +26,39 @@ impl NotificationRegistry { } /// Detect the first available notification backend. + /// + /// Returns the first backend (in registration order) that reports + /// `is_available() == true`. Registration order in `new()` therefore + /// determines priority. fn detect(&self) -> Option<&dyn NotificationBackend> { - self.backends.iter().find_map(|b| { - if b.is_available() { - Some(b.as_ref()) - } else { - None - } - }) + self.backends + .iter() + .find(|b| b.is_available()) + .map(|b| b.as_ref()) } } -/// Detect the first available notification backend for the current platform. -/// -/// Returns `None` on unsupported platforms or when no notification tools are installed. -pub fn detect_backend() -> Option<&'static dyn NotificationBackend> { - REGISTRY.detect() -} - /// Send a notification via the first available platform backend. /// -/// Best-effort: returns the backend's `Result` to let callers decide how -/// to handle failures. Returns `Ok(())` with a debug log if no backend is available. -pub fn send_via_backend(title: &str, message: &str) -> Result<(), super::errors::NotifyError> { - let Some(backend) = detect_backend() else { +/// Returns `Ok(true)` if the notification was sent, `Ok(false)` if no backend +/// is available (skipped), or `Err` if the backend failed. +pub fn send_via_backend(title: &str, message: &str) -> Result { + let Some(backend) = REGISTRY.detect() else { debug!( event = "core.notify.send_skipped", reason = "no backend available", ); - return Ok(()); + return Ok(false); }; - backend.send(title, message) + backend.send(title, message)?; + Ok(true) } #[cfg(test)] mod tests { use super::*; - #[test] - fn detect_backend_does_not_panic() { - // Should never panic regardless of platform - let _result = detect_backend(); - } - #[test] fn registry_contains_expected_backends() { let registry = NotificationRegistry::new(); @@ -80,7 +69,6 @@ mod tests { #[test] fn send_via_backend_does_not_panic() { - // Best-effort: should never panic even if no backend available let _result = send_via_backend("Test", "Hello"); } } diff --git a/crates/kild-core/src/process/types.rs b/crates/kild-core/src/process/types.rs index 855f121f..1c66fafd 100644 --- a/crates/kild-core/src/process/types.rs +++ b/crates/kild-core/src/process/types.rs @@ -118,7 +118,6 @@ mod tests { start_time: 1700001000, }; let _metadata: ProcessMetadata = (&info).into(); - // Original info still accessible assert_eq!(info.name, "kiro"); assert_eq!(info.pid.as_u32(), 5678); } diff --git a/crates/kild-core/src/sessions/types/status.rs b/crates/kild-core/src/sessions/types/status.rs index fefbd962..0b94676a 100644 --- a/crates/kild-core/src/sessions/types/status.rs +++ b/crates/kild-core/src/sessions/types/status.rs @@ -1,5 +1,6 @@ use kild_protocol::AgentStatus; use serde::{Deserialize, Serialize}; +use tracing::warn; #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] @@ -26,7 +27,13 @@ impl From for SessionStatus { kild_protocol::SessionStatus::Stopped => SessionStatus::Stopped, // SessionStatus is #[non_exhaustive]; treat unknown variants as Active // (alive until proven otherwise). - _ => SessionStatus::Active, + _ => { + warn!( + event = "core.session.status_conversion_unknown", + "Unknown daemon SessionStatus variant; treating as Active" + ); + SessionStatus::Active + } } } }