From 3dce7cf224c4f02ddd592cdf4157a87f3a96590a Mon Sep 17 00:00:00 2001 From: Henry Park Date: Fri, 27 Mar 2026 14:48:26 -0700 Subject: [PATCH] Stabilize routine verification state --- src/agent/routine.rs | 114 +++++++++++++++++++++++--- src/channels/web/handlers/routines.rs | 19 +++-- src/channels/web/static/app.js | 6 +- src/channels/web/types.rs | 26 +++++- src/tools/builtin/routine.rs | 13 ++- tests/gateway_workflow_integration.rs | 16 +++- 6 files changed, 166 insertions(+), 28 deletions(-) diff --git a/src/agent/routine.rs b/src/agent/routine.rs index 08e23f3d..0bea751d 100644 --- a/src/agent/routine.rs +++ b/src/agent/routine.rs @@ -591,8 +591,28 @@ fn write_routine_verification_record( Value::Object(obj) } +fn canonicalize_json_value(value: Value) -> Value { + match value { + Value::Array(items) => { + Value::Array(items.into_iter().map(canonicalize_json_value).collect()) + } + Value::Object(obj) => { + let mut keys: Vec = obj.keys().cloned().collect(); + keys.sort(); + let mut canonical = Map::new(); + for key in keys { + if let Some(value) = obj.get(&key) { + canonical.insert(key, canonicalize_json_value(value.clone())); + } + } + Value::Object(canonical) + } + other => other, + } +} + pub fn routine_verification_fingerprint(routine: &Routine) -> String { - let canonical = serde_json::json!({ + let canonical = canonicalize_json_value(serde_json::json!({ "trigger_type": routine.trigger.type_tag(), "trigger": routine.trigger.to_config_json(), "action_type": routine.action.type_tag(), @@ -602,7 +622,7 @@ pub fn routine_verification_fingerprint(routine: &Routine) -> String { "max_concurrent": routine.guardrails.max_concurrent, "dedup_window_secs": routine.guardrails.dedup_window.map(|d| d.as_secs()), }, - }) + })) .to_string(); let mut hasher = Sha256::new(); hasher.update(canonical.as_bytes()); @@ -628,17 +648,25 @@ pub fn apply_routine_verification_result( status: RunStatus, now: DateTime, ) -> serde_json::Value { - let mut record = routine_verification_record(state).unwrap_or(RoutineVerificationRecord { - current_fingerprint: current_fingerprint.clone(), - verified_fingerprint: None, - last_verified_at: None, - }); - record.current_fingerprint = current_fingerprint.clone(); - if status == RunStatus::Ok { - record.verified_fingerprint = Some(current_fingerprint); - record.last_verified_at = Some(now); + if let Some(mut record) = routine_verification_record(state) { + record.current_fingerprint = current_fingerprint.clone(); + if status == RunStatus::Ok { + record.verified_fingerprint = Some(current_fingerprint); + record.last_verified_at = Some(now); + } + write_routine_verification_record(state, record) + } else if status == RunStatus::Ok { + write_routine_verification_record( + state, + RoutineVerificationRecord { + current_fingerprint: current_fingerprint.clone(), + verified_fingerprint: Some(current_fingerprint), + last_verified_at: Some(now), + }, + ) + } else { + state.clone() } - write_routine_verification_record(state, record) } pub fn routine_verification_status(routine: &Routine) -> RoutineVerificationStatus { @@ -658,6 +686,18 @@ pub fn routine_verification_status(routine: &Routine) -> RoutineVerificationStat pub fn routine_display_status( routine: &Routine, last_run_status: Option, +) -> RoutineDisplayStatus { + routine_display_status_for_verification( + routine, + routine_verification_status(routine), + last_run_status, + ) +} + +pub fn routine_display_status_for_verification( + routine: &Routine, + verification_status: RoutineVerificationStatus, + last_run_status: Option, ) -> RoutineDisplayStatus { if !routine.enabled { return RoutineDisplayStatus::Disabled; @@ -665,7 +705,7 @@ pub fn routine_display_status( if last_run_status == Some(RunStatus::Running) { return RoutineDisplayStatus::Running; } - if routine_verification_status(routine) == RoutineVerificationStatus::Unverified { + if verification_status == RoutineVerificationStatus::Unverified { return RoutineDisplayStatus::Unverified; } if routine.consecutive_failures > 0 { @@ -1059,6 +1099,36 @@ mod tests { assert!(!fingerprint.contains("super-secret-routine-prompt")); } + #[test] + fn test_system_event_fingerprint_is_stable_when_filter_insertion_order_differs() { + let mut first_filters = std::collections::HashMap::new(); + first_filters.insert("repo".to_string(), "nearai/ironclaw".to_string()); + first_filters.insert("action".to_string(), "opened".to_string()); + + let mut second_filters = std::collections::HashMap::new(); + second_filters.insert("action".to_string(), "opened".to_string()); + second_filters.insert("repo".to_string(), "nearai/ironclaw".to_string()); + + let mut first = make_verification_test_routine(); + first.trigger = Trigger::SystemEvent { + source: "github".to_string(), + event_type: "issue".to_string(), + filters: first_filters, + }; + + let mut second = make_verification_test_routine(); + second.trigger = Trigger::SystemEvent { + source: "github".to_string(), + event_type: "issue".to_string(), + filters: second_filters, + }; + + assert_eq!( + routine_verification_fingerprint(&first), + routine_verification_fingerprint(&second) + ); + } + #[test] fn test_next_cron_fire_valid() { // Every minute should always have a next fire @@ -1466,4 +1536,22 @@ mod tests { RoutineVerificationStatus::Verified ); } + + #[test] + fn test_failed_legacy_run_preserves_implicit_verification() { + let mut routine = make_verification_test_routine(); + routine.run_count = 2; + let fingerprint = routine_verification_fingerprint(&routine); + routine.state = apply_routine_verification_result( + &routine.state, + fingerprint, + RunStatus::Failed, + Utc::now(), + ); + + assert_eq!( + routine_verification_status(&routine), + RoutineVerificationStatus::Verified + ); + } } diff --git a/src/channels/web/handlers/routines.rs b/src/channels/web/handlers/routines.rs index b139af21..edea92ff 100644 --- a/src/channels/web/handlers/routines.rs +++ b/src/channels/web/handlers/routines.rs @@ -11,7 +11,8 @@ use serde::Deserialize; use uuid::Uuid; use crate::agent::routine::{ - RoutineDisplayStatus, Trigger, next_cron_fire, routine_display_status, + RoutineDisplayStatus, RoutineVerificationStatus, Trigger, next_cron_fire, + routine_display_status_for_verification, routine_verification_status, }; use crate::channels::web::auth::AuthenticatedUser; use crate::channels::web::server::GatewayState; @@ -75,16 +76,24 @@ pub async fn routines_summary_handler( let mut failing = 0u64; for routine in &routines { + let verification_status = routine_verification_status(routine); if routine.enabled { enabled += 1; } else { disabled += 1; } - match routine_display_status(routine, last_run_statuses.get(&routine.id).copied()) { - RoutineDisplayStatus::Unverified => unverified += 1, - RoutineDisplayStatus::Failing => failing += 1, - _ => {} + if verification_status == RoutineVerificationStatus::Unverified { + unverified += 1; + } + + if routine_display_status_for_verification( + routine, + verification_status, + last_run_statuses.get(&routine.id).copied(), + ) == RoutineDisplayStatus::Failing + { + failing += 1; } } diff --git a/src/channels/web/static/app.js b/src/channels/web/static/app.js index 41e7ba25..e7271b54 100644 --- a/src/channels/web/static/app.js +++ b/src/channels/web/static/app.js @@ -4169,7 +4169,9 @@ function renderRoutinesList(routines) { const triggerTitle = (r.trigger_type === 'cron' && r.trigger_raw) ? ' title="' + escapeHtml(r.trigger_raw) + '"' : ''; - const runLabel = r.status === 'unverified' ? 'Verify now' : 'Run'; + const runLabel = (r.verification_status === 'unverified' || r.status === 'unverified') + ? 'Verify now' + : 'Run'; return '' + '' + escapeHtml(r.name) + '' @@ -4240,7 +4242,7 @@ function renderRoutineDetail(routine) { + '
' + escapeHtml(routine.description) + '
'; } - if (routine.status === 'unverified') { + if (routine.verification_status === 'unverified') { let verificationCopy = 'Created or updated, but not yet verified with a successful run.'; if (routine.recent_runs && routine.recent_runs.length > 0) { const latestRun = routine.recent_runs[0]; diff --git a/src/channels/web/types.rs b/src/channels/web/types.rs index fd2144b9..308b4cc2 100644 --- a/src/channels/web/types.rs +++ b/src/channels/web/types.rs @@ -714,8 +714,13 @@ impl RoutineInfo { crate::agent::routine::RoutineAction::FullJob { .. } => "full_job", }; - let status = crate::agent::routine::routine_display_status(r, last_run_status).as_str(); - let verification_status = crate::agent::routine::routine_verification_status(r).as_str(); + let verification_status = crate::agent::routine::routine_verification_status(r); + let status = crate::agent::routine::routine_display_status_for_verification( + r, + verification_status, + last_run_status, + ) + .as_str(); RoutineInfo { id: r.id, @@ -731,7 +736,7 @@ impl RoutineInfo { run_count: r.run_count, consecutive_failures: r.consecutive_failures, status: status.to_string(), - verification_status: verification_status.to_string(), + verification_status: verification_status.as_str().to_string(), } } } @@ -1288,4 +1293,19 @@ mod tests { assert_eq!(info.status, "active"); assert_eq!(info.verification_status, "verified"); } + + #[test] + fn test_routine_info_keeps_unverified_state_when_disabled() { + let mut routine = make_routine_for_status_tests(); + routine.state = crate::agent::routine::reset_routine_verification_state( + &routine.state, + crate::agent::routine::routine_verification_fingerprint(&routine), + ); + routine.enabled = false; + + let info = RoutineInfo::from_routine(&routine, None); + + assert_eq!(info.status, "disabled"); + assert_eq!(info.verification_status, "unverified"); + } } diff --git a/src/tools/builtin/routine.rs b/src/tools/builtin/routine.rs index fe29f2da..3010a0cf 100644 --- a/src/tools/builtin/routine.rs +++ b/src/tools/builtin/routine.rs @@ -20,8 +20,8 @@ use uuid::Uuid; use crate::agent::routine::{ NotifyConfig, Routine, RoutineAction, RoutineGuardrails, Trigger, next_cron_fire, - normalize_cron_expression, reset_routine_verification_state, routine_display_status, - routine_verification_fingerprint, routine_verification_status, + normalize_cron_expression, reset_routine_verification_state, routine_verification_fingerprint, + routine_verification_status, }; use crate::agent::routine_engine::RoutineEngine; use crate::context::JobContext; @@ -1243,7 +1243,12 @@ impl Tool for RoutineListTool { let list: Vec = routines .iter() .map(|r| { - let status = routine_display_status(r, last_run_statuses.get(&r.id).copied()); + let verification_status = routine_verification_status(r); + let status = crate::agent::routine::routine_display_status_for_verification( + r, + verification_status, + last_run_statuses.get(&r.id).copied(), + ); serde_json::json!({ "id": r.id.to_string(), "name": r.name, @@ -1256,7 +1261,7 @@ impl Tool for RoutineListTool { "run_count": r.run_count, "consecutive_failures": r.consecutive_failures, "status": status.as_str(), - "verification_status": routine_verification_status(r).as_str(), + "verification_status": verification_status.as_str(), }) }) .collect(); diff --git a/tests/gateway_workflow_integration.rs b/tests/gateway_workflow_integration.rs index 534a202a..70793995 100644 --- a/tests/gateway_workflow_integration.rs +++ b/tests/gateway_workflow_integration.rs @@ -389,6 +389,20 @@ mod tests { .await .expect("create routine"); + let mut disabled_routine = routine.clone(); + disabled_routine.id = Uuid::new_v4(); + disabled_routine.name = "wf-unverified-disabled".to_string(); + disabled_routine.enabled = false; + disabled_routine.state = reset_routine_verification_state( + &disabled_routine.state, + routine_verification_fingerprint(&disabled_routine), + ); + harness + .db + .create_routine(&disabled_routine) + .await + .expect("create disabled routine"); + let list = harness.list_routines().await; let routine_id = routine.id.to_string(); let listed = list["routines"] @@ -412,7 +426,7 @@ mod tests { .json::() .await .expect("invalid summary response"); - assert_eq!(summary["unverified"].as_u64(), Some(1)); + assert_eq!(summary["unverified"].as_u64(), Some(2)); let detail = harness .client