mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-25 14:53:34 +00:00
Stabilize routine verification state
This commit is contained in:
+101
-13
@@ -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<String> = 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<Utc>,
|
||||
) -> 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<RunStatus>,
|
||||
) -> 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<RunStatus>,
|
||||
) -> 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
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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 '<tr class="routine-row" data-action="open-routine" data-id="' + escapeHtml(r.id) + '">'
|
||||
+ '<td>' + escapeHtml(r.name) + '</td>'
|
||||
@@ -4240,7 +4242,7 @@ function renderRoutineDetail(routine) {
|
||||
+ '<div class="job-description-body">' + escapeHtml(routine.description) + '</div></div>';
|
||||
}
|
||||
|
||||
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];
|
||||
|
||||
@@ -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");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<serde_json::Value> = 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();
|
||||
|
||||
@@ -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::<serde_json::Value>()
|
||||
.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
|
||||
|
||||
Reference in New Issue
Block a user