From fd166d1ebbaf3bcab8a65369d78d6b07b174b868 Mon Sep 17 00:00:00 2001 From: Felix Martin Date: Tue, 26 May 2026 09:57:01 -0400 Subject: [PATCH] Fix violation prompt invariants --- src/main.rs | 122 ++++++++++++++++++++--- src/session.rs | 263 +++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 351 insertions(+), 34 deletions(-) diff --git a/src/main.rs b/src/main.rs index 4420f8e..5de44e6 100644 --- a/src/main.rs +++ b/src/main.rs @@ -9,6 +9,7 @@ use std::time::{Duration, Instant}; // <--- Add this mod constants; use antidrift::{ + context::{window_class_allowed, window_title_allowed}, domain::{EvidenceHealth, PolicySnapshot, RuntimeState}, session::SessionController, window, @@ -94,6 +95,21 @@ struct SessionRating { rating: u8, } +#[derive(Clone, Debug, PartialEq, Eq)] +struct WindowIdentity { + title: String, + class: Option, +} + +impl From<&window::WindowSnapshot> for WindowIdentity { + fn from(snapshot: &window::WindowSnapshot) -> Self { + Self { + title: snapshot.title.clone(), + class: snapshot.class.clone(), + } + } +} + struct App { state: State, user_intention: String, @@ -115,6 +131,7 @@ struct App { session_ratings: Vec, session_ratings_index: usize, session_results: Vec, + acknowledged_unknown_window: Option, last_tick_50ms: Instant, last_tick_1s: Instant, } @@ -203,6 +220,7 @@ impl App { session_ratings: Vec::new(), session_ratings_index: 0, session_results: Vec::new(), + acknowledged_unknown_window: None, last_tick_50ms: now, last_tick_1s: now, }) @@ -313,6 +331,7 @@ impl App { "evidence_health": self.evidence_health, }), )?; + self.acknowledge_unknown_window(&window::get_snapshot()); self.state = State::InProgress; Ok(()) } @@ -429,13 +448,15 @@ impl App { self.session_remaining = self.user_duration.saturating_sub(elapsed); let window_snapshot = update_session_stats(self); - if self.state == State::InProgress + let should_prompt = self.state == State::InProgress && self.session_controller.runtime_state() == RuntimeState::Active + && !self.is_unknown_window_acknowledged(&window_snapshot) && self .session_controller .active_policy() - .is_some_and(|policy| should_prompt_for_unknown_window(policy, &window_snapshot)) - { + .is_some_and(|policy| should_prompt_for_unknown_window(policy, &window_snapshot)); + + if should_prompt { self.enter_violation_prompt(&window_snapshot); return Ok(()); } @@ -456,6 +477,20 @@ impl App { } } + fn acknowledge_unknown_window(&mut self, window_snapshot: &window::WindowSnapshot) { + self.acknowledged_unknown_window = Some(WindowIdentity::from(window_snapshot)); + } + + fn is_unknown_window_acknowledged(&mut self, window_snapshot: &window::WindowSnapshot) -> bool { + let current = WindowIdentity::from(window_snapshot); + if self.acknowledged_unknown_window.as_ref() == Some(¤t) { + return true; + } + + self.acknowledged_unknown_window = None; + false + } + fn timeout(&self) -> Duration { Duration::from_millis(50).saturating_sub(self.last_tick_50ms.elapsed()) } @@ -560,17 +595,11 @@ fn should_prompt_for_unknown_window( return false; } - let title = window_snapshot.title.to_lowercase(); - let title_matches = allowed_context - .window_title_substrings - .iter() - .any(|allowed| title.contains(&allowed.to_lowercase())); - let class_matches = window_snapshot.class.as_ref().is_some_and(|class| { - allowed_context - .window_classes - .iter() - .any(|allowed| class.eq_ignore_ascii_case(allowed)) - }); + let title_matches = window_title_allowed(allowed_context, &window_snapshot.title); + let class_matches = window_snapshot + .class + .as_ref() + .is_some_and(|class| window_class_allowed(allowed_context, class)); !(title_matches || class_matches) } @@ -1108,6 +1137,46 @@ mod tests { assert!(!should_prompt_for_unknown_window(&policy, &snapshot)); } + #[test] + fn unknown_window_prompt_uses_shared_title_normalization() { + let policy = PolicySnapshot::for_runtime( + "commitment-test".to_string(), + RuntimeState::Active, + EnforcementLevel::Warn, + AllowedContext { + window_title_substrings: vec![" Editor ".to_string()], + ..AllowedContext::default() + }, + ); + let snapshot = window::WindowSnapshot { + title: "Code Editor - antidrift".to_string(), + class: None, + health: antidrift::domain::EvidenceHealth::Available, + }; + + assert!(!should_prompt_for_unknown_window(&policy, &snapshot)); + } + + #[test] + fn unknown_window_prompt_uses_shared_class_normalization() { + let policy = PolicySnapshot::for_runtime( + "commitment-test".to_string(), + RuntimeState::Active, + EnforcementLevel::Warn, + AllowedContext { + window_classes: vec![" Code ".to_string()], + ..AllowedContext::default() + }, + ); + let snapshot = window::WindowSnapshot { + title: "Unrelated title".to_string(), + class: Some(" code ".to_string()), + health: antidrift::domain::EvidenceHealth::Available, + }; + + assert!(!should_prompt_for_unknown_window(&policy, &snapshot)); + } + #[test] fn unknown_window_prompt_is_active_for_nonmatching_title() { let policy = PolicySnapshot::for_runtime( @@ -1128,6 +1197,31 @@ mod tests { assert!(should_prompt_for_unknown_window(&policy, &snapshot)); } + #[test] + fn acknowledged_unknown_window_is_suppressed_until_window_changes() { + let path = unique_event_log_path("violation-ack-suppression"); + let mut app = App::new_with_event_log_path(&path).unwrap(); + let unknown = window::WindowSnapshot { + title: "AntiDrift".to_string(), + class: Some("terminal".to_string()), + health: antidrift::domain::EvidenceHealth::Available, + }; + let other_unknown = window::WindowSnapshot { + title: "Browser - unrelated".to_string(), + class: Some("firefox".to_string()), + health: antidrift::domain::EvidenceHealth::Available, + }; + + assert!(!app.is_unknown_window_acknowledged(&unknown)); + app.acknowledge_unknown_window(&unknown); + + assert!(app.is_unknown_window_acknowledged(&unknown)); + assert!(!app.is_unknown_window_acknowledged(&other_unknown)); + assert!(!app.is_unknown_window_acknowledged(&unknown)); + + let _ = fs::remove_file(path); + } + #[test] fn violation_prompt_requires_reason_then_logs_and_returns_to_progress() { let path = unique_event_log_path("violation-prompt-dismissal"); diff --git a/src/session.rs b/src/session.rs index 2003a05..1147b4a 100644 --- a/src/session.rs +++ b/src/session.rs @@ -540,14 +540,21 @@ impl SessionController { evidence, }; validate_violation_observed_payload(&payload)?; + if self.runtime_state != RuntimeState::Active { + return Err(anyhow!("violation requires active runtime")); + } + let commitment_id = self + .active_commitment + .as_ref() + .ok_or_else(|| anyhow!("violation requires active commitment"))? + .id + .clone(); self.event_log .append( EventType::ViolationObserved, - self.runtime_state, - self.active_commitment - .as_ref() - .map(|commitment| commitment.id.clone()), + RuntimeState::Active, + Some(commitment_id), serde_json::to_value(payload).context("serialize violation observed payload")?, ) .context("log violation observation")?; @@ -873,26 +880,38 @@ fn hydrate_session( record.sequence ) })?; - if record.runtime_state != runtime_state { + if runtime_state != RuntimeState::Active + || record.runtime_state != RuntimeState::Active + { return Err(anyhow!( - "violation runtime mismatch at sequence {}", + "violation requires active runtime at sequence {}", record.sequence )); } - if let Some(commitment_id) = record.commitment_id.as_deref() { - let active = active_commitment.as_ref().ok_or_else(|| { - anyhow!( - "violation references unknown commitment '{}' at sequence {}", - commitment_id, - record.sequence - ) - })?; - if active.id != commitment_id { - return Err(anyhow!( - "violation commitment mismatch at sequence {}", - record.sequence - )); - } + let commitment_id = record.commitment_id.as_deref().ok_or_else(|| { + anyhow!( + "violation requires active commitment at sequence {}", + record.sequence + ) + })?; + let active = active_commitment.as_ref().ok_or_else(|| { + anyhow!( + "violation references unknown commitment '{}' at sequence {}", + commitment_id, + record.sequence + ) + })?; + if active.state != CommitmentState::Active { + return Err(anyhow!( + "violation requires active commitment at sequence {}", + record.sequence + )); + } + if active.id != commitment_id { + return Err(anyhow!( + "violation commitment mismatch at sequence {}", + record.sequence + )); } } ref unsupported => { @@ -1311,6 +1330,210 @@ mod tests { fs::remove_file(path).unwrap(); } + #[test] + fn record_violation_requires_active_runtime_and_commitment() { + let planning_path = temp_log_path("violation-planning"); + let mut planning = SessionController::new(&planning_path).unwrap(); + planning.enter_planning().unwrap(); + + let err = planning + .record_violation( + "Needed to inspect docs", + json!({"message": "Unknown context detected"}), + ) + .expect_err("planning violations must be rejected"); + assert!(error_chain_contains(&err, "active")); + fs::remove_file(planning_path).unwrap(); + + let transition_path = temp_log_path("violation-transition"); + let mut transition = SessionController::new(&transition_path).unwrap(); + transition.enter_planning().unwrap(); + transition + .start_manual_commitment( + "Implement session controller", + "session tests pass", + Duration::from_secs(1500), + ) + .unwrap(); + transition + .start_transition( + "Need to inspect failures", + "Return to session controller", + Duration::from_secs(300), + ) + .unwrap(); + + let err = transition + .record_violation( + "Needed to inspect docs", + json!({"message": "Unknown context detected"}), + ) + .expect_err("transition violations must be rejected"); + assert!(error_chain_contains(&err, "active")); + fs::remove_file(transition_path).unwrap(); + + let review_path = temp_log_path("violation-review"); + let mut review = SessionController::new(&review_path).unwrap(); + review.enter_planning().unwrap(); + review + .start_manual_commitment( + "Implement session controller", + "session tests pass", + Duration::from_secs(1500), + ) + .unwrap(); + review.complete_for_review().unwrap(); + + let err = review + .record_violation( + "Needed to inspect docs", + json!({"message": "Unknown context detected"}), + ) + .expect_err("review violations must be rejected"); + assert!(error_chain_contains(&err, "active")); + fs::remove_file(review_path).unwrap(); + } + + #[test] + fn replay_rejects_violation_without_active_commitment() { + let path = temp_log_path("violation-replay-no-commitment"); + { + let mut log = EventLog::open(&path).unwrap(); + log.append_batch([ + PendingEventRecord { + event_type: EventType::RuntimeTransition, + runtime_state: RuntimeState::Planning, + commitment_id: None, + payload_json: json!({ + "schema_version": POLICY_SCHEMA_VERSION, + "action": "enter_planning", + "from": "locked", + "to": "planning", + }), + }, + PendingEventRecord { + event_type: EventType::ViolationObserved, + runtime_state: RuntimeState::Planning, + commitment_id: None, + payload_json: json!({ + "schema_version": POLICY_SCHEMA_VERSION, + "reason": "Needed to inspect docs", + "evidence": {"message": "Unknown context detected"}, + }), + }, + ]) + .unwrap(); + } + + let err = SessionController::new(&path) + .expect_err("planning violation without commitment must be rejected"); + + assert!(error_chain_contains(&err, "active")); + fs::remove_file(path).unwrap(); + } + + #[test] + fn replay_rejects_active_violation_without_commitment_id() { + let path = temp_log_path("violation-replay-missing-commitment"); + let commitment_id = "commitment-test"; + { + let mut log = EventLog::open(&path).unwrap(); + log.append_batch([ + PendingEventRecord { + event_type: EventType::RuntimeTransition, + runtime_state: RuntimeState::Planning, + commitment_id: None, + payload_json: json!({ + "schema_version": POLICY_SCHEMA_VERSION, + "action": "enter_planning", + "from": "locked", + "to": "planning", + }), + }, + PendingEventRecord { + event_type: EventType::CommitmentCreated, + runtime_state: RuntimeState::Active, + commitment_id: Some(commitment_id.to_string()), + payload_json: commitment_payload(commitment_id, CommitmentState::Active), + }, + PendingEventRecord { + event_type: EventType::PolicyApplied, + runtime_state: RuntimeState::Active, + commitment_id: Some(commitment_id.to_string()), + payload_json: policy_payload(commitment_id), + }, + PendingEventRecord { + event_type: EventType::ViolationObserved, + runtime_state: RuntimeState::Active, + commitment_id: None, + payload_json: json!({ + "schema_version": POLICY_SCHEMA_VERSION, + "reason": "Needed to inspect docs", + "evidence": {"message": "Unknown context detected"}, + }), + }, + ]) + .unwrap(); + } + + let err = SessionController::new(&path) + .expect_err("active violation without commitment must be rejected"); + + assert!(error_chain_contains(&err, "active commitment")); + fs::remove_file(path).unwrap(); + } + + #[test] + fn replay_rejects_violation_for_wrong_commitment() { + let path = temp_log_path("violation-replay-wrong-commitment"); + let commitment_id = "commitment-test"; + { + let mut log = EventLog::open(&path).unwrap(); + log.append_batch([ + PendingEventRecord { + event_type: EventType::RuntimeTransition, + runtime_state: RuntimeState::Planning, + commitment_id: None, + payload_json: json!({ + "schema_version": POLICY_SCHEMA_VERSION, + "action": "enter_planning", + "from": "locked", + "to": "planning", + }), + }, + PendingEventRecord { + event_type: EventType::CommitmentCreated, + runtime_state: RuntimeState::Active, + commitment_id: Some(commitment_id.to_string()), + payload_json: commitment_payload(commitment_id, CommitmentState::Active), + }, + PendingEventRecord { + event_type: EventType::PolicyApplied, + runtime_state: RuntimeState::Active, + commitment_id: Some(commitment_id.to_string()), + payload_json: policy_payload(commitment_id), + }, + PendingEventRecord { + event_type: EventType::ViolationObserved, + runtime_state: RuntimeState::Active, + commitment_id: Some("commitment-other".to_string()), + payload_json: json!({ + "schema_version": POLICY_SCHEMA_VERSION, + "reason": "Needed to inspect docs", + "evidence": {"message": "Unknown context detected"}, + }), + }, + ]) + .unwrap(); + } + + let err = + SessionController::new(&path).expect_err("wrong violation commitment must be rejected"); + + assert!(error_chain_contains(&err, "commitment mismatch")); + fs::remove_file(path).unwrap(); + } + #[test] fn activation_batch_emits_adjacent_stable_payloads() { let path = temp_log_path("activation-batch");