mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-25 14:53:34 +00:00
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
This commit is contained in:
@@ -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<bool>,
|
||||
}
|
||||
|
||||
async fn routines_toggle_handler(
|
||||
State(state): State<Arc<GatewayState>>,
|
||||
Path(id): Path<String>,
|
||||
body: Option<Json<ToggleRequest>>,
|
||||
) -> Result<Json<serde_json::Value>, (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<Arc<GatewayState>>,
|
||||
Path(id): Path<String>,
|
||||
) -> Result<Json<serde_json::Value>, (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<Arc<GatewayState>>,
|
||||
Path(id): Path<String>,
|
||||
|
||||
@@ -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<RoutineEngine>, Arc<dyn Database>, 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"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user