diff --git a/crates/local-deployment/src/container.rs b/crates/local-deployment/src/container.rs index 677f1aff..48f36afc 100644 --- a/crates/local-deployment/src/container.rs +++ b/crates/local-deployment/src/container.rs @@ -52,6 +52,7 @@ use services::services::{ diff_stream::{self, DiffStreamHandle}, git::{Commit, DiffTarget, GitService}, image::ImageService, + notification::NotificationService, queued_message::QueuedMessageService, share::SharePublisher, worktree_manager::{WorktreeCleanup, WorktreeManager}, @@ -80,6 +81,7 @@ pub struct LocalContainerService { approvals: Approvals, queued_message_service: QueuedMessageService, publisher: Result, + notification_service: NotificationService, } impl LocalContainerService { @@ -97,6 +99,7 @@ impl LocalContainerService { ) -> Self { let child_store = Arc::new(RwLock::new(HashMap::new())); let interrupt_senders = Arc::new(RwLock::new(HashMap::new())); + let notification_service = NotificationService::new(config.clone()); let container = LocalContainerService { db, @@ -110,6 +113,7 @@ impl LocalContainerService { approvals, queued_message_service, publisher, + notification_service, }; container.spawn_worktree_cleanup().await; @@ -424,9 +428,7 @@ impl LocalContainerService { ); // Manually finalize task since we're bypassing normal execution flow - container - .finalize_task(&config, publisher.as_ref().ok(), &ctx) - .await; + container.finalize_task(publisher.as_ref().ok(), &ctx).await; } } @@ -469,9 +471,7 @@ impl LocalContainerService { { tracing::error!("Failed to start queued follow-up: {}", e); // Fall back to finalization if follow-up fails - container - .finalize_task(&config, publisher.as_ref().ok(), &ctx) - .await; + container.finalize_task(publisher.as_ref().ok(), &ctx).await; } } else { // Execution failed or was killed - discard the queued message and finalize @@ -480,14 +480,10 @@ impl LocalContainerService { ctx.task_attempt.id, ctx.execution_process.status ); - container - .finalize_task(&config, publisher.as_ref().ok(), &ctx) - .await; + container.finalize_task(publisher.as_ref().ok(), &ctx).await; } } else { - container - .finalize_task(&config, publisher.as_ref().ok(), &ctx) - .await; + container.finalize_task(publisher.as_ref().ok(), &ctx).await; } } @@ -819,6 +815,10 @@ impl ContainerService for LocalContainerService { self.publisher.as_ref().ok() } + fn notification_service(&self) -> &NotificationService { + &self.notification_service + } + async fn git_branch_prefix(&self) -> String { self.config.read().await.git_branch_prefix.clone() } @@ -976,6 +976,7 @@ impl ContainerService for LocalContainerService { ExecutorApprovalBridge::new( self.approvals.clone(), self.db.clone(), + self.notification_service.clone(), execution_process.id, ) } diff --git a/crates/services/src/services/approvals/executor_approvals.rs b/crates/services/src/services/approvals/executor_approvals.rs index dbbd9882..fb8e096b 100644 --- a/crates/services/src/services/approvals/executor_approvals.rs +++ b/crates/services/src/services/approvals/executor_approvals.rs @@ -7,19 +7,26 @@ use serde_json::Value; use utils::approvals::{ApprovalRequest, ApprovalStatus, CreateApprovalRequest}; use uuid::Uuid; -use crate::services::approvals::Approvals; +use crate::services::{approvals::Approvals, notification::NotificationService}; pub struct ExecutorApprovalBridge { approvals: Approvals, db: DBService, + notification_service: NotificationService, execution_process_id: Uuid, } impl ExecutorApprovalBridge { - pub fn new(approvals: Approvals, db: DBService, execution_process_id: Uuid) -> Arc { + pub fn new( + approvals: Approvals, + db: DBService, + notification_service: NotificationService, + execution_process_id: Uuid, + ) -> Arc { Arc::new(Self { approvals, db, + notification_service, execution_process_id, }) } @@ -50,6 +57,14 @@ impl ExecutorApprovalService for ExecutorApprovalBridge { .await .map_err(ExecutorApprovalError::request_failed)?; + // Play notification sound when approval is needed + self.notification_service + .notify( + "Approval Needed", + &format!("Tool '{}' requires approval", tool_name), + ) + .await; + let status = waiter.clone().await; if matches!(status, ApprovalStatus::Pending) { diff --git a/crates/services/src/services/container.rs b/crates/services/src/services/container.rs index 38f78ccf..5d17f30b 100644 --- a/crates/services/src/services/container.rs +++ b/crates/services/src/services/container.rs @@ -41,7 +41,6 @@ use utils::{ use uuid::Uuid; use crate::services::{ - config::Config, git::{GitService, GitServiceError}, notification::NotificationService, share::SharePublisher, @@ -79,6 +78,8 @@ pub trait ContainerService { fn share_publisher(&self) -> Option<&SharePublisher>; + fn notification_service(&self) -> &NotificationService; + fn task_attempt_to_current_dir(&self, task_attempt: &TaskAttempt) -> PathBuf; async fn create(&self, task_attempt: &TaskAttempt) -> Result; @@ -161,7 +162,6 @@ pub trait ContainerService { /// Finalize task execution by updating status to InReview and sending notifications async fn finalize_task( &self, - config: &Arc>, share_publisher: Option<&SharePublisher>, ctx: &ExecutionContext, ) { @@ -181,8 +181,31 @@ pub trait ContainerService { tracing::error!("Failed to update task status to InReview: {e}"); } } - let notify_cfg = config.read().await.notifications.clone(); - NotificationService::notify_execution_halted(notify_cfg, ctx).await; + + // Skip notification if process was intentionally killed by user + if matches!(ctx.execution_process.status, ExecutionProcessStatus::Killed) { + return; + } + + let title = format!("Task Complete: {}", ctx.task.title); + let message = match ctx.execution_process.status { + ExecutionProcessStatus::Completed => format!( + "✅ '{}' completed successfully\nBranch: {:?}\nExecutor: {}", + ctx.task.title, ctx.task_attempt.branch, ctx.task_attempt.executor + ), + ExecutionProcessStatus::Failed => format!( + "❌ '{}' execution failed\nBranch: {:?}\nExecutor: {}", + ctx.task.title, ctx.task_attempt.branch, ctx.task_attempt.executor + ), + _ => { + tracing::warn!( + "Tried to notify attempt completion for {} but process is still running!", + ctx.task_attempt.id + ); + return; + } + }; + self.notification_service().notify(&title, &message).await; } /// Cleanup executions marked as running in the db, call at startup diff --git a/crates/services/src/services/notification.rs b/crates/services/src/services/notification.rs index 931a35fd..3e7f70a1 100644 --- a/crates/services/src/services/notification.rs +++ b/crates/services/src/services/notification.rs @@ -1,52 +1,32 @@ -use std::sync::OnceLock; +use std::sync::{Arc, OnceLock}; -use db::models::execution_process::{ExecutionContext, ExecutionProcessStatus}; +use tokio::sync::RwLock; use utils; -use crate::services::config::SoundFile; +use crate::services::config::{Config, NotificationConfig, SoundFile}; /// Service for handling cross-platform notifications including sound alerts and push notifications #[derive(Debug, Clone)] -pub struct NotificationService {} -use crate::services::config::NotificationConfig; +pub struct NotificationService { + config: Arc>, +} /// Cache for WSL root path from PowerShell static WSL_ROOT_PATH_CACHE: OnceLock> = OnceLock::new(); impl NotificationService { - pub async fn notify_execution_halted(mut config: NotificationConfig, ctx: &ExecutionContext) { - // If the process was intentionally killed by user, suppress sound - if matches!(ctx.execution_process.status, ExecutionProcessStatus::Killed) { - config.sound_enabled = false; - } - - let title = format!("Task Complete: {}", ctx.task.title); - let message = match ctx.execution_process.status { - ExecutionProcessStatus::Completed => format!( - "✅ '{}' completed successfully\nBranch: {:?}\nExecutor: {}", - ctx.task.title, ctx.task_attempt.branch, ctx.task_attempt.executor - ), - ExecutionProcessStatus::Failed => format!( - "❌ '{}' execution failed\nBranch: {:?}\nExecutor: {}", - ctx.task.title, ctx.task_attempt.branch, ctx.task_attempt.executor - ), - ExecutionProcessStatus::Killed => format!( - "🛑 '{}' execution cancelled by user\nBranch: {:?}\nExecutor: {}", - ctx.task.title, ctx.task_attempt.branch, ctx.task_attempt.executor - ), - _ => { - tracing::warn!( - "Tried to notify attempt completion for {} but process is still running!", - ctx.task_attempt.id - ); - return; - } - }; - Self::notify(config, &title, &message).await; + pub fn new(config: Arc>) -> Self { + Self { config } } /// Send both sound and push notifications if enabled - pub async fn notify(config: NotificationConfig, title: &str, message: &str) { + pub async fn notify(&self, title: &str, message: &str) { + let config = self.config.read().await.notifications.clone(); + Self::send_notification(&config, title, message).await; + } + + /// Internal method to send notifications with a given config + async fn send_notification(config: &NotificationConfig, title: &str, message: &str) { if config.sound_enabled { Self::play_sound_notification(&config.sound_file).await; }