From 971b4c2ef43872d87dfbcbecce2587761c1dd860 Mon Sep 17 00:00:00 2001 From: Nick Pismenkov <50764773+nickpismenkov@users.noreply.github.com> Date: Mon, 16 Mar 2026 13:16:35 -0700 Subject: [PATCH] fix: web/CLI routine mutations do not refresh live event trigger cache (#1255) * fix: web/CLI routine mutations do not refresh live event trigger cache * review fix --- src/channels/web/server.rs | 79 +---------------------- tests/e2e_routine_heartbeat.rs | 114 +++++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 78 deletions(-) diff --git a/src/channels/web/server.rs b/src/channels/web/server.rs index fb8c93ae..1eb49e3c 100644 --- a/src/channels/web/server.rs +++ b/src/channels/web/server.rs @@ -26,7 +26,6 @@ use tower_http::set_header::SetResponseHeaderLayer; use uuid::Uuid; use crate::agent::SessionManager; -use crate::agent::routine::{Trigger, next_cron_fire}; use crate::bootstrap::ironclaw_base_dir; use crate::channels::IncomingMessage; use crate::channels::relay::DEFAULT_RELAY_NAME; @@ -36,6 +35,7 @@ use crate::channels::web::handlers::jobs::{ jobs_events_handler, jobs_list_handler, jobs_prompt_handler, jobs_restart_handler, jobs_summary_handler, }; +use crate::channels::web::handlers::routines::{routines_delete_handler, routines_toggle_handler}; use crate::channels::web::handlers::skills::{ skills_install_handler, skills_list_handler, skills_remove_handler, skills_search_handler, }; @@ -2470,83 +2470,6 @@ async fn routines_trigger_handler( }))) } -#[derive(Deserialize)] -struct ToggleRequest { - enabled: Option, -} - -async fn routines_toggle_handler( - State(state): State>, - Path(id): Path, - body: Option>, -) -> Result, (StatusCode, String)> { - let store = state.store.as_ref().ok_or(( - StatusCode::SERVICE_UNAVAILABLE, - "Database not available".to_string(), - ))?; - - let routine_id = Uuid::parse_str(&id) - .map_err(|_| (StatusCode::BAD_REQUEST, "Invalid routine ID".to_string()))?; - - let mut routine = store - .get_routine(routine_id) - .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))? - .ok_or((StatusCode::NOT_FOUND, "Routine not found".to_string()))?; - - let was_enabled = routine.enabled; - // If a specific value was provided, use it; otherwise toggle. - routine.enabled = match body { - Some(Json(req)) => req.enabled.unwrap_or(!routine.enabled), - None => !routine.enabled, - }; - - if routine.enabled - && !was_enabled - && let Trigger::Cron { schedule, timezone } = &routine.trigger - { - routine.next_fire_at = next_cron_fire(schedule, timezone.as_deref()) - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; - } - - store - .update_routine(&routine) - .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; - - Ok(Json(serde_json::json!({ - "status": if routine.enabled { "enabled" } else { "disabled" }, - "routine_id": routine_id, - }))) -} - -async fn routines_delete_handler( - State(state): State>, - Path(id): Path, -) -> Result, (StatusCode, String)> { - let store = state.store.as_ref().ok_or(( - StatusCode::SERVICE_UNAVAILABLE, - "Database not available".to_string(), - ))?; - - let routine_id = Uuid::parse_str(&id) - .map_err(|_| (StatusCode::BAD_REQUEST, "Invalid routine ID".to_string()))?; - - let deleted = store - .delete_routine(routine_id) - .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; - - if deleted { - Ok(Json(serde_json::json!({ - "status": "deleted", - "routine_id": routine_id, - }))) - } else { - Err((StatusCode::NOT_FOUND, "Routine not found".to_string())) - } -} - async fn routines_runs_handler( State(state): State>, Path(id): Path, diff --git a/tests/e2e_routine_heartbeat.rs b/tests/e2e_routine_heartbeat.rs index 6d6deb8b..1ee8d389 100644 --- a/tests/e2e_routine_heartbeat.rs +++ b/tests/e2e_routine_heartbeat.rs @@ -553,4 +553,118 @@ mod tests { "Expected Skipped for empty checklist, got: {result:?}" ); } + + /// Helper to set up a test environment for routine engine mutation tests. + /// Returns the engine, database, and temp directory. + async fn setup_routine_mutation_test() + -> (Arc, Arc, tempfile::TempDir) { + let (db, dir) = create_test_db().await; + let ws = create_workspace(&db); + let (notify_tx, _rx) = tokio::sync::mpsc::channel(16); + let tools = Arc::new(ToolRegistry::new()); + + let safety_config = SafetyConfig { + max_output_length: 100_000, + injection_check_enabled: true, + }; + let safety = Arc::new(SafetyLayer::new(&safety_config)); + + let trace = LlmTrace::single_turn( + "test-routine-mutation", + "test", + vec![TraceStep { + request_hint: None, + response: TraceResponse::Text { + content: "ROUTINE_OK".to_string(), + input_tokens: 50, + output_tokens: 5, + }, + expected_tool_results: vec![], + }], + ); + let llm = Arc::new(TraceLlm::from_trace(trace)); + + let engine = Arc::new(RoutineEngine::new( + RoutineConfig::default(), + Arc::clone(&db), + llm, + ws, + notify_tx, + None, + tools, + safety, + )); + + (engine, db, dir) + } + + /// Regression test for issue #1076: disabling an event routine via a DB mutation + /// followed by refresh_event_cache() (the path now taken by the web toggle handler) + /// must immediately stop the routine from firing. + #[tokio::test] + async fn toggle_disabling_event_routine_removes_from_cache() { + let (engine, db, _dir) = setup_routine_mutation_test().await; + + // Create and cache an event routine. + let mut routine = make_routine( + "disable-me", + Trigger::Event { + pattern: "DISABLE_ME".to_string(), + channel: None, + }, + "Handle DISABLE_ME event", + ); + db.create_routine(&routine).await.expect("create_routine"); + engine.refresh_event_cache().await; + + let msg = IncomingMessage::new("test", "default", "DISABLE_ME"); + let fired_before = engine.check_event_triggers(&msg).await; + assert!(fired_before >= 1, "Expected routine to fire before disable"); + + // Simulate what routines_toggle_handler now does: update DB, then refresh. + routine.enabled = false; + routine.updated_at = Utc::now(); + db.update_routine(&routine).await.expect("update_routine"); + engine.refresh_event_cache().await; + + let fired_after = engine.check_event_triggers(&msg).await; + assert_eq!( + fired_after, 0, + "Disabled routine must not fire after cache refresh" + ); + } + + /// Regression test for issue #1076: deleting an event routine via a DB mutation + /// followed by refresh_event_cache() must immediately stop the routine from firing. + #[tokio::test] + async fn delete_event_routine_removes_from_cache() { + let (engine, db, _dir) = setup_routine_mutation_test().await; + + let routine = make_routine( + "delete-me", + Trigger::Event { + pattern: "DELETE_ME".to_string(), + channel: None, + }, + "Handle DELETE_ME event", + ); + db.create_routine(&routine).await.expect("create_routine"); + engine.refresh_event_cache().await; + + let msg = IncomingMessage::new("test", "default", "DELETE_ME"); + assert!( + engine.check_event_triggers(&msg).await >= 1, + "Expected routine to fire before delete" + ); + + // Simulate what routines_delete_handler now does: delete from DB, then refresh. + db.delete_routine(routine.id).await.expect("delete_routine"); + engine.refresh_event_cache().await; + + assert_eq!( + engine.check_event_triggers(&msg).await, + 0, + "Deleted routine must not fire after cache refresh" + ); + } }