diff --git a/src-tauri/src/automation/roster.rs b/src-tauri/src/automation/roster.rs index 4d18f5ef..d953efbe 100644 --- a/src-tauri/src/automation/roster.rs +++ b/src-tauri/src/automation/roster.rs @@ -54,7 +54,7 @@ pub struct TargetSnapshot { } /// One picker row. -#[derive(Debug, Clone, PartialEq, serde::Serialize)] +#[derive(Debug, Clone, PartialEq, serde::Serialize, serde::Deserialize)] #[serde(rename_all = "camelCase")] pub struct WatchableTerminal { pub terminal_id: String, @@ -63,6 +63,17 @@ pub struct WatchableTerminal { pub shell: Option, pub pid: Option, pub cwd: Option, + /// The tab/pane label the targeting resolver reads for `Tab name contains`. This is not the + /// terminal's internal `name`: renderer-created terminals all share a `Terminal-{shell}` name, + /// so that would make a shell name look like a tab title. + pub display_label: Option, + /// The foreground process chain the targeting resolver reads for `Command contains`. + /// + /// The editor sends its roster back to the preview command, rather than implementing a second + /// matcher in TypeScript. Keeping the chain on this DTO is what lets that command use the same + /// resolver as the evaluator. + #[serde(default)] + pub command_lines: Vec, /// `false` means "not open right now" — **dormant, never dead**. Session restore re-registers the /// same `tm-` under a new `pc-`, so absence is not death. pub alive: bool, @@ -101,6 +112,8 @@ pub fn build( shell: Some(row.shell.clone()), pid: Some(row.pid), cwd: row.cwd.clone().or_else(|| snap.and_then(|s| s.folder.clone())), + display_label: row.display_label.clone(), + command_lines: row.command_lines.clone(), alive: true, }); } @@ -117,6 +130,8 @@ pub fn build( shell: None, pid: None, cwd: snap.and_then(|s| s.folder.clone()), + display_label: None, + command_lines: Vec::new(), alive: false, }); } diff --git a/src-tauri/src/automation/runtime.rs b/src-tauri/src/automation/runtime.rs index 9bfdc4c1..f2b462f8 100644 --- a/src-tauri/src/automation/runtime.rs +++ b/src-tauri/src/automation/runtime.rs @@ -589,6 +589,16 @@ impl AutomationRuntime { self.watched.get(rule_id).map(|e| e.value().clone()).unwrap_or_default() } + /// Has the targeting loop resolved this rule's matched set even once? + /// + /// `watched_for` cannot answer this: it flattens a MISSING entry and an entry holding an empty + /// set into the same empty set, and those two mean opposite things to the row that reads them. + /// The targeting pass calls `set_watched` for every live rule, empty result included, so an + /// absent key means "not resolved yet" and only that. + pub fn has_resolved(&self, rule_id: &str) -> bool { + self.watched.contains_key(rule_id) + } + pub fn watches(&self, rule_id: &str, tm: &str) -> bool { self.watched.get(rule_id).map(|e| e.value().contains(tm)).unwrap_or(false) } diff --git a/src-tauri/src/automation/targeting.rs b/src-tauri/src/automation/targeting.rs index 07748175..3c97e959 100644 --- a/src-tauri/src/automation/targeting.rs +++ b/src-tauri/src/automation/targeting.rs @@ -121,6 +121,50 @@ pub fn resolve(criterion: Criterion, value: &str, rows: &[RosterRow]) -> BTreeSe .collect() } +/// The three target sets the editor needs to explain a rule. +/// +/// `matched` is the unfiltered target set, `excluded` is the part of that set the rule filters +/// out, and `watching` is the remainder. They are sets rather than counts because matching the +/// wrong terminal can leave every displayed count unchanged. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct TargetResolution { + pub matched: BTreeSet, + pub excluded: BTreeSet, + pub watching: BTreeSet, +} + +/// Resolve one rule's target sets for both the evaluator and the editor preview. +/// +/// `previous` preserves the evaluator's frozen-set behaviour. The preview passes `None`, which +/// answers what the draft resolves to now; the evaluator passes its stored set. Keeping those two +/// callers here gives them one matcher and one exclusion gate. +pub fn resolve_target_sets( + rule: &AutomationRule, + rows: &[RosterRow], + previous: Option<&BTreeSet>, +) -> TargetResolution { + let matched = match rule.target_mode { + TargetMode::Pinned => rule.target_ids.iter().cloned().collect(), + TargetMode::Rule => match (rule.follow_new, previous) { + (false, Some(frozen)) if !frozen.is_empty() => frozen.clone(), + _ => resolve(rule.criterion, &rule.criterion_value, rows), + }, + }; + + let excluded = if rule.target_mode == TargetMode::Rule { + let mut candidates: BTreeSet = rule.excluded_ids.iter().cloned().collect(); + if let Some(criterion) = rule.exclude_criterion { + candidates.extend(resolve(criterion, &rule.exclude_criterion_value, rows)); + } + matched.intersection(&candidates).cloned().collect() + } else { + BTreeSet::new() + }; + let watching = matched.difference(&excluded).cloned().collect(); + + TargetResolution { matched, excluded, watching } +} + /// What changed between two ticks of the watched set. #[derive(Debug, Default, Clone, PartialEq, Eq)] pub struct SetDelta { @@ -165,13 +209,7 @@ pub fn watched_set( rows: &[RosterRow], previous: Option<&BTreeSet>, ) -> BTreeSet { - match rule.target_mode { - TargetMode::Pinned => rule.target_ids.iter().cloned().collect(), - TargetMode::Rule => match (rule.follow_new, previous) { - (false, Some(frozen)) if !frozen.is_empty() => frozen.clone(), - _ => resolve(rule.criterion, &rule.criterion_value, rows), - }, - } + resolve_target_sets(rule, rows, previous).watching } #[cfg(test)] @@ -466,6 +504,9 @@ mod tests { criterion_value: value.into(), follow_new, target_ids: vec!["tm-pinned".into()], + excluded_ids: vec![], + exclude_criterion: None, + exclude_criterion_value: String::new(), completed_at: None, verbose_until: None, sort_order: 0, @@ -473,7 +514,7 @@ mod tests { graph: AutomationGraph { layout: None, timer: None, - monitor: Some(MonitorStep { read: ReadMode::NewOutput, cadence: Cadence::OnOutput, every_ms: 0 }), + monitor: Some(MonitorStep { read: ReadMode::NewOutput, cadence: Cadence::OnOutput, every_ms: 0, skip_typed_line: false }), parse: Some(ParseStep { preset: ParsePreset::Custom, literal: None, @@ -481,13 +522,14 @@ mod tests { keep: Keep::Brackets, }), cond: Some(CondStep { finds: Finds::Reading, op: Some(CompareOp::Gt), threshold: Some(25.0), ..Default::default() }), - action: ActionStep { + action: Some(ActionStep { message: "m".into(), send_to: SendTo::Matched, submit: true, cli_type: "default".into(), substitute: false, - }, + }), + webhook: None, }, created_at: 0, updated_at: 0, @@ -565,6 +607,70 @@ mod tests { assert_eq!(ids(watched_set(&pinned, &[], None)), vec!["tm-pinned"]); } + #[test] + fn a_pinned_rule_ignores_exclusions_entirely() { + let rows = vec![row(Some("tm-a"), None, None, Some("claude"))]; + let mut r = rule(TargetMode::Pinned, Criterion::CommandContains, "claude", true); + r.target_ids = vec!["tm-a".into()]; + r.excluded_ids = vec!["tm-a".into()]; + assert_eq!( + ids(watched_set(&r, &rows, None)), + vec!["tm-a"], + "a hand-picked set says what it means; exclusions do not apply" + ); + } + + #[test] + fn an_exclusion_pattern_removes_everything_it_matches() { + let rows = vec![ + row(Some("tm-scratch"), None, Some("~/scratch/project"), None), + row(Some("tm-work"), None, Some("~/work/termflow"), None), + ]; + let mut r = rule(TargetMode::Rule, Criterion::AllTerminals, "", true); + r.exclude_criterion = Some(Criterion::WorkingFolderUnder); + r.exclude_criterion_value = "~/scratch".into(); + + assert_eq!(ids(watched_set(&r, &rows, None)), vec!["tm-work"]); + } + + #[test] + fn the_two_exclusion_kinds_union_rather_than_override() { + let rows = vec![ + row(Some("tm-id"), None, Some("~/work/termflow"), None), + row(Some("tm-pattern"), None, Some("~/scratch/project"), None), + row(Some("tm-kept"), None, Some("~/work/other"), None), + ]; + let mut r = rule(TargetMode::Rule, Criterion::AllTerminals, "", true); + r.excluded_ids = vec!["tm-id".into()]; + r.exclude_criterion = Some(Criterion::WorkingFolderUnder); + r.exclude_criterion_value = "~/scratch".into(); + + assert_eq!(ids(watched_set(&r, &rows, None)), vec!["tm-kept"]); + } + + /// Counts alone let the editor and the engine disagree while both "pass": excluding tm-b + /// instead of tm-c gives the same three numbers. Assert the SETS. + #[test] + fn the_preview_resolves_the_same_ids_the_engine_watches() { + let rows = [ + row(Some("tm-a"), None, None, Some("node worker-a")), + row(Some("tm-b"), None, None, Some("node worker-b")), + row(Some("tm-c"), None, None, Some("node worker-c")), + ]; + let mut r = rule(TargetMode::Rule, Criterion::CommandContains, "node", true); + r.excluded_ids = vec!["tm-b".into()]; + + let preview = resolve_target_sets(&r, &rows, None); + assert_eq!(ids(preview.matched), vec!["tm-a", "tm-b", "tm-c"]); + assert_eq!(ids(preview.excluded), vec!["tm-b"]); + assert_eq!(ids(preview.watching.clone()), vec!["tm-a", "tm-c"]); + assert_eq!( + ids(watched_set(&r, &rows, None)), + ids(preview.watching), + "the evaluator must use the preview's resolution, not merely have the same count" + ); + } + // ----------------------------------------------------------------------------------------- // §10.12b — the departure is reported once // ----------------------------------------------------------------------------------------- diff --git a/src-tauri/src/automation_commands.rs b/src-tauri/src/automation_commands.rs index b173ce48..7787b3cb 100644 --- a/src-tauri/src/automation_commands.rs +++ b/src-tauri/src/automation_commands.rs @@ -25,6 +25,7 @@ use tauri::{Emitter, State}; use crate::automation::events::{ChangedPayload, AUTOMATION_CHANGED}; use crate::automation::roster::{TargetSnapshot, WatchableTerminal}; +use crate::automation::targeting::resolve_target_sets; use crate::automation_engine::dry::DryRunReport; use crate::automation_engine::host::EngineHost; use crate::automation_store::{ @@ -43,6 +44,13 @@ fn to_string_err(e: AutomationStoreError) -> String { e.to_string() } +/// Decode an IPC rule without forwarding serde's rendering of the submitted value. A malformed +/// webhook object may contain its endpoint in that rendering, while callers only need to know that +/// the draft was malformed. +fn decode_automation_rule(value: serde_json::Value) -> Result { + serde_json::from_value(value).map_err(|_| "automation rule is malformed".to_string()) +} + /// One rule-level log row — no terminal, so no name to carry. /// /// **The three definition kinds had no writer at all.** `Saved` is required by §3.5 and checked by GUI @@ -175,20 +183,15 @@ pub async fn list_watchable_terminals( state: State<'_, AppState>, rule_id: Option, include_ids: Option>, + criteria: Vec, ) -> Result, String> { let owned = state.inner().clone(); tokio::task::spawn_blocking(move || { let store = owned.automation_store.clone(); - // Only the criterion of the rule being edited, so opening the picker on a `Terminal ID is` - // rule never enumerates the machine's processes (§10.13). - let criteria: Vec = match rule_id.as_deref() { - Some(id) => store - .get_rule(id) - .map_err(to_string_err)? - .map(|r| vec![r.criterion]) - .unwrap_or_default(), - None => Vec::new(), - }; + // The editor supplies its CURRENT rule-mode criteria, including the exception. `rule_id` + // scopes only the persisted label snapshots below: a saved row is stale while its draft is + // being edited, and a new draft has no saved row at all. Keeping those jobs separate means + // a command/cwd criterion gets the scan it needs before preview resolution. let live = EngineHost::roster(&owned, &criteria); // §4.3: scoped to the rule when the caller names one — the editor always knows which rule it @@ -219,6 +222,48 @@ pub async fn list_watchable_terminals( .map_err(|e| e.to_string())? } +/// The ids a draft targets right now, split into its unfiltered match, the part excluded, and what +/// remains to watch. The renderer supplies the roster it is drawing so the preview and its picker +/// describe one snapshot; matching itself remains in Rust alongside the evaluator. +#[derive(Debug, Clone, serde::Serialize)] +#[serde(rename_all = "camelCase")] +pub struct AutomationTargetPreview { + pub matched: Vec, + pub excluded: Vec, + pub watching: Vec, +} + +#[tauri::command] +pub async fn preview_automation_targets( + rule: AutomationRule, + terminals: Vec, +) -> Result { + tokio::task::spawn_blocking(move || { + let rows = terminals + .into_iter() + .filter(|terminal| terminal.alive) + .map(|terminal| crate::automation::roster::RosterRow { + terminal_id: Some(terminal.terminal_id), + process_id: terminal.process_id.unwrap_or_default(), + name: String::new(), + shell: terminal.shell.unwrap_or_default(), + pid: terminal.pid.unwrap_or_default(), + display_label: terminal.display_label, + cwd: terminal.cwd, + command_lines: terminal.command_lines, + }) + .collect::>(); + let resolved = resolve_target_sets(&rule, &rows, None); + Ok(AutomationTargetPreview { + matched: resolved.matched.into_iter().collect(), + excluded: resolved.excluded.into_iter().collect(), + watching: resolved.watching.into_iter().collect(), + }) + }) + .await + .map_err(|e| e.to_string())? +} + /// Which pairs `Re-arm now` touches: one named leaf, or every leaf the rule watches. /// /// **A named leaf is filtered through the watch set too.** It arrives from a renderer that may be @@ -260,9 +305,10 @@ fn reload_after_commit(owned: &AppState, at: i64) -> Result<(), String> { #[tauri::command] pub async fn dry_run_automation( state: State<'_, AppState>, - rule: AutomationRule, + rule: serde_json::Value, terminal_id: String, ) -> Result { + let rule = decode_automation_rule(rule)?; let owned = state.inner().clone(); tokio::task::spawn_blocking(move || { let engine = owned.automations.clone(); @@ -301,11 +347,11 @@ pub struct AutomationSaveResult { #[tauri::command] pub async fn save_automation( state: State<'_, AppState>, - rule: AutomationRule, + rule: serde_json::Value, origin: String, ) -> Result { let owned = state.inner().clone(); - let mut rule = rule; + let mut rule = decode_automation_rule(rule)?; if rule.id.trim().is_empty() { // The same shape `duplicate_automation` mints, and deliberately the same prefix: one id // vocabulary, minted in one crate. @@ -650,7 +696,46 @@ pub async fn rearm_automation( #[cfg(test)] mod source_tests { - use super::leaves_to_rearm; + use super::{decode_automation_rule, leaves_to_rearm}; + + #[test] + fn malformed_ipc_rule_errors_do_not_echo_a_webhook_url() { + let secret = "https://hooks.example.invalid/ipc-credential"; + let value = serde_json::json!({ + "id": "au-ipc", + "name": "bad webhook", + "enabled": false, + "runsOnce": false, + "targetMode": "pinned", + "criterion": "allTerminals", + "criterionValue": "", + "followNew": true, + "targetIds": [], + "excludedIds": [], + "completedAt": null, + "verboseUntil": null, + "sortOrder": 0, + "schemaVersion": 3, + "graph": { "webhook": { + "provider": secret, + "url": secret, + "body": "done" + }}, + "createdAt": 0, + "updatedAt": 0 + }); + + // This is the exact serde decode that Tauri used to perform for the command argument. + // Its real error includes the invalid provider, so accepting a typed parameter leaked it. + let raw = serde_json::from_value::(value.clone()) + .expect_err("a URL is not a webhook provider") + .to_string(); + assert!(raw.contains(secret), "premise: raw IPC decode did not contain the URL: {raw}"); + + let safe = decode_automation_rule(value).expect_err("the command rejects malformed IPC"); + assert_eq!(safe, "automation rule is malformed"); + assert!(!safe.contains(secret)); + } /// `Re-arm now` reaches only pairs the rule actually watches. /// @@ -890,7 +975,7 @@ mod source_tests { fn every_command_in_this_module_is_registered_in_lib() { let lib = crate::automation_engine::test_host::strip_comments(include_str!("lib.rs")); let names: Vec = command_bodies().into_iter().map(|(n, _)| n).collect(); - assert_eq!(names.len(), 14, "the command list changed: {:?}", names); + assert_eq!(names.len(), 15, "the command list changed: {:?}", names); for name in &names { assert!( lib.contains(&format!("automation_commands::{},", name)), diff --git a/src-tauri/src/automation_engine.rs b/src-tauri/src/automation_engine.rs index 521cbe45..83f786fc 100644 --- a/src-tauri/src/automation_engine.rs +++ b/src-tauri/src/automation_engine.rs @@ -773,8 +773,28 @@ impl AutomationEngine { let mut rules = HashMap::new(); for live in self.snapshot_live() { let id = &live.rule.id; - let watched = self.runtime.watched_for(id); let missing_for = missing.get(id).unwrap_or(&empty); + // **A rule that has never been resolved is absent, not a rule watching nothing.** + // + // The row treats these as opposite: an absent rule is `waiting` ("the engine has not + // reported this rule"), an empty one is the *Nothing to watch* error ("running, and + // nothing matches"). Every live rule was reported here from the moment it went live, + // and `watched_for` returns an empty set for a rule the targeting loop has not reached + // yet — so between a reload and the next targeting pass, up to `TARGETING_TICK_MS`, + // every rule-mode rule claimed nothing matched it. + // + // That window opens on the two occasions a user is most likely to be looking: app + // start, and the reload that follows their own save. Saving a `Command contains` rule + // with a matching terminal already open showed *"No open terminal matches ..."* for two + // seconds, and the pill was `Error` while it did. + // + // `missing` is parked by the same pass, so it cannot be present while `watched` is + // absent — but reporting the rule when it somehow is loses nothing, and hiding a known + // missing terminal would. + if !self.runtime.has_resolved(id) && missing_for.is_empty() { + continue; + } + let watched = self.runtime.watched_for(id); let mut pairs = HashMap::new(); for tm in watched.iter().chain(missing_for.iter()) { let (fired_count, last_fired_at) = match self.runtime.fire_record(id, tm) { @@ -840,6 +860,9 @@ mod tests { criterion_value: String::new(), follow_new: true, target_ids: vec!["tm-1".to_string()], + excluded_ids: vec![], + exclude_criterion: None, + exclude_criterion_value: String::new(), completed_at: None, verbose_until: None, sort_order: 1, @@ -851,6 +874,7 @@ mod tests { read: ReadMode::NewOutput, cadence: Cadence::OnOutput, every_ms: 0, + skip_typed_line: false, }), parse: Some(ParseStep { preset: ParsePreset::Custom, @@ -864,13 +888,14 @@ mod tests { threshold: Some(25.0), ..Default::default() }), - action: ActionStep { + action: Some(ActionStep { message: "prepare to do context-hand-off".to_string(), send_to: SendTo::Matched, submit: true, cli_type: "default".to_string(), substitute: false, - }, + }), + webhook: None, }, created_at: 1_000, updated_at: 1_000, @@ -991,6 +1016,25 @@ mod tests { assert!(rows[1].contains("needs a newer version"), "{}", rows[1]); } + #[test] + fn reload_logs_a_real_malformed_webhook_value_without_its_url() { + let secret = "https://hooks.example.invalid/reload-credential"; + let malformed = format!( + r#"{{"webhook":{{"provider":"{secret}","url":"{secret}","body":"done"}}}}"# + ); + let store = AutomationStore::new_in_memory(); + store.insert_raw_graph_for_test("au-malformed", &malformed); + + let engine = AutomationEngine::new(0); + let report = engine.reload(&store, 7_000).expect("reload malformed row"); + assert_eq!(report.skipped.len(), 1, "the malformed row was really skipped"); + assert!(!report.skipped[0].1.contains(secret), "reload reason leaked: {:?}", report.skipped); + + let rows = log_rows(&store); + assert_eq!(rows.len(), 1, "reload wrote its real activity row"); + assert!(!rows[0].contains(secret), "activity detail leaked: {}", rows[0]); + } + /// **A schedule rule has no pattern, and no pattern is not a broken pattern** (plan 032 §6.4). /// /// `reload` used to ask `pattern_refused_at_load(&graph.parse.find)` of every rule, and a rule @@ -1967,6 +2011,67 @@ mod tests { assert_eq!(engine.runtime.arm_state("au-a", "tm-1"), ArmState::Fired { at_ms: 500 }); } + /// "Un-ticking puts it back." An exclusion is a filter over the matched set, never a deletion from + /// it — and this must hold on a FROZEN (`follow_new: false`) rule, which is the case that can bake + /// the exclusion in. Spec §B3. Distinct timestamps are load-bearing: reload compares `updated_at`, + /// not content, so a same-millisecond save would not clear the set and the test would pass + /// vacuously. + #[test] + fn lifting_an_exclusion_restores_the_terminal_on_a_frozen_rule() { + let fake = Arc::new( + crate::automation_engine::test_host::FakeHost::new() + .with_terminal("tm-a", "pc-a", "a") + .with_terminal("tm-b", "pc-b", "b"), + ); + let host: Arc = fake.clone(); + let engine = Arc::new(AutomationEngine::new(0)); + + let mut r = rule("au-frozen", r"ctx:(\d+)%"); + r.target_mode = TargetMode::Rule; + r.criterion = Criterion::AllTerminals; + r.criterion_value.clear(); + r.follow_new = false; + r.updated_at = 1_000; + fake.store.save_rule(&r).unwrap(); + engine.reload(&fake.store, 1_000).unwrap(); + crate::automation_engine::loops::targeting_tick(&engine, &host, 1_000); + assert_eq!( + engine.runtime.watched_for("au-frozen"), + HashSet::from(["tm-a".to_string(), "tm-b".to_string()]), + "premise: the frozen base set contains both terminals" + ); + + r.excluded_ids = vec!["tm-b".into()]; + r.updated_at = 2_000; + fake.store.save_rule(&r).unwrap(); + engine.reload(&fake.store, 2_000).unwrap(); + assert!( + engine.runtime.watched_for("au-frozen").is_empty(), + "the changed timestamp must clear the frozen set before the next targeting pass" + ); + crate::automation_engine::loops::targeting_tick(&engine, &host, 2_000); + assert_eq!( + engine.runtime.watched_for("au-frozen"), + HashSet::from(["tm-a".to_string()]), + "the exclusion filters tm-b from the frozen base set" + ); + + r.excluded_ids.clear(); + r.updated_at = 3_000; + fake.store.save_rule(&r).unwrap(); + engine.reload(&fake.store, 3_000).unwrap(); + assert!( + engine.runtime.watched_for("au-frozen").is_empty(), + "lifting the exclusion must also clear the filtered frozen set" + ); + crate::automation_engine::loops::targeting_tick(&engine, &host, 3_000); + assert_eq!( + engine.runtime.watched_for("au-frozen"), + HashSet::from(["tm-a".to_string(), "tm-b".to_string()]), + "the original matched terminal returns after a real save and reload" + ); + } + // ----------------------------------------------------------------------------------------- // §7.8 — completion is an in-memory event first // ----------------------------------------------------------------------------------------- @@ -2030,6 +2135,41 @@ mod tests { assert!(!p.missing); } + /// The three states of a rule's matched set, as a table — because two of them are an empty map + /// and the row treats them as opposites. + /// + /// Never resolved => ABSENT, which the row reads as `waiting`. Resolved to nothing => PRESENT + /// and empty, which is the *Nothing to watch* error. Resolved to something => the pairs. + /// + /// The middle row is the one that makes the first row safe: hiding an unresolved rule must not + /// also hide a rule that genuinely matches no terminal, or the error becomes unreachable. + #[test] + fn a_rule_is_absent_until_targeting_resolves_it_and_empty_only_when_nothing_matches() { + let store = AutomationStore::new_in_memory(); + store.save_rule(&rule("au-a", r"ctx:(\d+)%")).unwrap(); + let engine = AutomationEngine::new(0); + engine.reload(&store, 1_000).unwrap(); + + assert!( + !engine.state_payload(&HashMap::new()).rules.contains_key("au-a"), + "a live rule the targeting loop has not reached yet must not claim nothing matches it", + ); + + engine.runtime.set_watched("au-a", HashSet::new()); + let resolved_to_nothing = engine.state_payload(&HashMap::new()); + assert_eq!( + resolved_to_nothing.rules.get("au-a").map(|p| p.len()), + Some(0), + "resolved-and-matching-nothing is still reported, as an empty map", + ); + + engine.runtime.set_watched("au-a", ["tm-1".to_string()].into()); + assert_eq!( + engine.state_payload(&HashMap::new()).rules["au-a"].len(), + 1, + ); + } + /// Every field moves for the right reason — and `fired_count` survives a re-arm, which is what /// `arm` alone structurally cannot express. #[test] diff --git a/src-tauri/src/automation_engine/dry.rs b/src-tauri/src/automation_engine/dry.rs index 0d699373..bfd7d47b 100644 --- a/src-tauri/src/automation_engine/dry.rs +++ b/src-tauri/src/automation_engine/dry.rs @@ -26,8 +26,8 @@ use crate::automation_engine::schedule::clock_time; use crate::automation_engine::subst; use crate::automation_engine::AutomationEngine; use crate::automation_store::{ - AutomationLogEntry, AutomationRule, Clause, CompareOp, Finds, Join, LogKind, Test, TextOp, - TimerMode, TimerStep, + ActionStep, AutomationLogEntry, AutomationRule, Clause, CompareOp, Finds, Join, LogKind, Test, TextOp, + TimerMode, TimerStep, WebhookStep, }; /// One step of the graph, as the editor's Test panel draws it. @@ -73,6 +73,7 @@ const PARSE: &str = "parse"; const COND: &str = "cond"; const TIMER: &str = "timer"; const ACTION: &str = "action"; +const WEBHOOK: &str = "webhook"; /// An `AfterMatch` wait, in the pane's own terse style — `30 s`, `1.5 s`, `2 min` — never restating /// `MAX_DELAY_MS`/`MIN_DELAY_MS` as a bound, because this is naming a CONCRETE wait, not a range. @@ -126,6 +127,56 @@ fn skipped(kind: &str) -> StepTrace { step(kind, "skipped", "not reached".to_string()) } +fn action_trace(action: Option<&ActionStep>, caps: Option<&eval::Captures>, terminal_name: Option<&str>) -> Option { + let action = action?; + Some(match preview_message(&action.message, action.substitute, caps) { + Ok(body) => step(ACTION, "ok", match terminal_name { + Some(name) => format!("would type `{}` into {}", body, name), + None => format!("would type `{}`", body), + }), + Err(e) => step(ACTION, "failed", format!("nothing would be sent — `{}` had no value here", e)), + }) +} + +fn push_skipped_action(steps: &mut Vec, action: Option<&ActionStep>) { + if action.is_some() { steps.push(skipped(ACTION)); } +} + +fn masked_webhook_url(url: &str) -> String { + match reqwest::Url::parse(url) { + Ok(url) => match url.host_str() { + Some(host) => format!("{}://{}", url.scheme(), host), + None => "an invalid webhook URL".to_string(), + }, + Err(_) => "an invalid webhook URL".to_string(), + } +} + +fn webhook_trace(webhook: Option<&WebhookStep>, caps: Option<&eval::Captures>) -> Option { + let webhook = webhook?; + Some(match preview_message(&webhook.body, webhook.substitute, caps) { + Ok(message) => { + let body = String::from_utf8(crate::automation_webhook::payload(webhook.provider, &message)) + .expect("a webhook payload from a UTF-8 string is UTF-8"); + step( + WEBHOOK, + "ok", + format!( + "would post via {:?} `{}` to {}", + webhook.provider, + body, + masked_webhook_url(&webhook.url), + ), + ) + } + Err(e) => step(WEBHOOK, "failed", format!("nothing would be sent — `{}` had no value here", e)), + }) +} + +fn push_skipped_webhook(steps: &mut Vec, webhook: Option<&WebhookStep>) { + if webhook.is_some() { steps.push(skipped(WEBHOOK)); } +} + fn symbol(op: CompareOp) -> &'static str { match op { CompareOp::Gt => ">", @@ -264,7 +315,10 @@ pub fn evaluate_once( // this branch still answers an unsaved draft or a row that predates the guard, so it // stays the honest, non-panicking answer regardless of whether validation would also // have refused it. - return finish(UNREADABLE, vec![skipped(TIMER), skipped(ACTION)]); + let mut steps = vec![skipped(TIMER)]; + push_skipped_action(&mut steps, rule.graph.action.as_ref()); + push_skipped_webhook(&mut steps, rule.graph.webhook.as_ref()); + return finish(UNREADABLE, steps); }; // **I1.** `schedule_due` refuses a `minute_of_day` outside `0..MINUTES_PER_DAY` and a // `days` mask with no weekday bit set — and until this check existed, this branch asked @@ -293,10 +347,7 @@ pub fn evaluate_once( }; return finish( WOULD_NOT_FIRE, - vec![ - step(TIMER, "failed", format!("this schedule can never fire — {why}")), - skipped(ACTION), - ], + { let mut steps = vec![step(TIMER, "failed", format!("this schedule can never fire — {why}"))]; push_skipped_action(&mut steps, rule.graph.action.as_ref()); push_skipped_webhook(&mut steps, rule.graph.webhook.as_ref()); steps }, ); } let timer_step = step( @@ -309,42 +360,29 @@ pub fn evaluate_once( let Some(_pc) = host.process_for_leaf(tm) else { return finish( UNREADABLE, - vec![timer_step, step(ACTION, "failed", "that terminal is not open right now".to_string())], + { let mut steps = vec![timer_step]; if rule.graph.action.is_some() { steps.push(step(ACTION, "failed", "that terminal is not open right now".to_string())); } push_skipped_webhook(&mut steps, rule.graph.webhook.as_ref()); steps }, ); }; - let action_step = match preview_message(&rule.graph.action.message, rule.graph.action.substitute, None) { - Ok(body) => step( - ACTION, - "ok", - match &terminal_name { - Some(name) => format!("would type `{}` into {}", body, name), - None => format!("would type `{}`", body), - }, - ), - Err(e) => step(ACTION, "failed", format!("nothing would be sent — `{}` had no value here", e)), - }; - return finish(WOULD_FIRE, vec![timer_step, action_step]); + let mut steps = vec![timer_step]; + if let Some(action_step) = action_trace(rule.graph.action.as_ref(), None, terminal_name.as_deref()) { steps.push(action_step); } + if let Some(webhook_step) = webhook_trace(rule.graph.webhook.as_ref(), None) { steps.push(webhook_step); } + return finish(WOULD_FIRE, steps); } let Some(steps) = eval::InputSteps::of(&rule.graph) else { return finish( UNREADABLE, - vec![skipped(MONITOR), skipped(PARSE), skipped(COND), skipped(ACTION)], + { let mut steps = vec![skipped(MONITOR), skipped(PARSE), skipped(COND)]; push_skipped_action(&mut steps, rule.graph.action.as_ref()); push_skipped_webhook(&mut steps, rule.graph.webhook.as_ref()); steps }, ); }; let pattern = steps.parse.find.clone(); // 1. Monitor. A terminal that is not live is not readable, and that is the whole answer. let Some(pc) = host.process_for_leaf(tm) else { - return finish( - UNREADABLE, - vec![ - step(MONITOR, "failed", "that terminal is not open right now".to_string()), - skipped(PARSE), - skipped(COND), - skipped(ACTION), - ], - ); + let mut steps = vec![step(MONITOR, "failed", "that terminal is not open right now".to_string()), skipped(PARSE), skipped(COND)]; + push_skipped_action(&mut steps, rule.graph.action.as_ref()); + push_skipped_webhook(&mut steps, rule.graph.webhook.as_ref()); + return finish(UNREADABLE, steps); }; // 2. The pattern has to compile before anything can be read for it. The editor's validation says @@ -352,9 +390,7 @@ pub fn evaluate_once( let re = match crate::automation_validation::compile(&pattern) { Ok(re) => re, Err(e) => { - return finish( - WOULD_NOT_FIRE, - vec![ + let mut steps = vec![ step(MONITOR, "ok", "the terminal is open".to_string()), step( PARSE, @@ -365,9 +401,10 @@ pub fn evaluate_once( ), ), skipped(COND), - skipped(ACTION), - ], - ); + ]; + push_skipped_action(&mut steps, rule.graph.action.as_ref()); + push_skipped_webhook(&mut steps, rule.graph.webhook.as_ref()); + return finish(WOULD_NOT_FIRE, steps); } }; @@ -398,15 +435,10 @@ pub fn evaluate_once( let echoes = engine.runtime.echoes_for(tm, now_ms); let port = HostPort(host); let Some(ev) = eval::evaluate(folded, &re, &echoes, prev, &port, &pc, now_ms) else { - return finish( - UNREADABLE, - vec![ - step(MONITOR, "failed", "there is nothing to read from that terminal yet".to_string()), - skipped(PARSE), - skipped(COND), - skipped(ACTION), - ], - ); + let mut steps = vec![step(MONITOR, "failed", "there is nothing to read from that terminal yet".to_string()), skipped(PARSE), skipped(COND)]; + push_skipped_action(&mut steps, rule.graph.action.as_ref()); + push_skipped_webhook(&mut steps, rule.graph.webhook.as_ref()); + return finish(UNREADABLE, steps); }; let monitor = step(MONITOR, "ok", format!("read {}", eval::depth_words(ev.depth))); @@ -518,30 +550,14 @@ pub fn evaluate_once( let would_fire = ev.condition == Truth::True; let action = if would_fire { - match preview_message( - &rule.graph.action.message, - rule.graph.action.substitute, - ev.captures.as_ref(), - ) { - Ok(body) => step( - ACTION, - "ok", - match &terminal_name { - Some(name) => format!("would type `{}` into {}", body, name), - None => format!("would type `{}`", body), - }, - ), - // §4.4: the same refusal `run_send` would make, reported rather than typed. The - // verdict below still answers the CONDITION (it matched), and this row alone carries - // that the send itself would be refused. - Err(e) => step( - ACTION, - "failed", - format!("nothing would be sent — `{}` had no value here", e), - ), - } + action_trace(rule.graph.action.as_ref(), ev.captures.as_ref(), terminal_name.as_deref()) } else { - step(ACTION, "skipped", "nothing would be sent".to_string()) + rule.graph.action.as_ref().map(|_| step(ACTION, "skipped", "nothing would be sent".to_string())) + }; + let webhook = if would_fire { + webhook_trace(rule.graph.webhook.as_ref(), ev.captures.as_ref()) + } else { + rule.graph.webhook.as_ref().map(|_| step(WEBHOOK, "skipped", "nothing would be sent".to_string())) }; // A DELAY rule (`AfterMatch`) inserts a `timer` row here, between the comparison and the send — @@ -565,7 +581,8 @@ pub fn evaluate_once( if let Some(t) = timer_step { all_steps.push(t); } - all_steps.push(action); + if let Some(action) = action { all_steps.push(action); } + if let Some(webhook) = webhook { all_steps.push(webhook); } finish(verdict, all_steps) } @@ -589,6 +606,7 @@ mod tests { use crate::automation_engine::test_host::*; use crate::automation_store::{ Clause, CondStep, Finds, Join, Keep, LogOrder, LogScope, Source, Test as ClauseTest, TextOp, + WebhookProvider, WebhookStep, }; fn kinds(report: &DryRunReport) -> Vec<&str> { @@ -1114,6 +1132,29 @@ mod tests { assert_eq!(report.terminal_id, "tm-1"); } + #[test] + fn the_webhook_step_names_its_provider_and_payload_without_its_secret_url_path() { + let (engine, fake, host) = wire(vec![]); + let mut rule = ctx_rule("au-1"); + rule.graph.webhook = Some(WebhookStep { + provider: WebhookProvider::Discord, + url: "https://hooks.example.invalid/secret-token".into(), + body: "build failed".into(), + substitute: false, + }); + fake.say("pc-1", "ctx:63%\n"); + + let report = evaluate_once(&engine, host.as_ref(), &rule, "tm-1", 1_000); + + assert_eq!(kinds(&report), vec!["monitor", "parse", "cond", "action", "webhook"]); + let webhook = detail(&report, "webhook"); + assert_eq!(status(&report, "webhook"), "ok"); + assert!(webhook.contains("Discord"), "{webhook}"); + assert!(webhook.contains(r#"{"content":"build failed"}"#), "{webhook}"); + assert!(webhook.contains("https://hooks.example.invalid"), "{webhook}"); + assert!(!webhook.contains("/secret-token"), "{webhook}"); + } + // ============================================================================================= // §1.1 — the preview must not disagree with the send // ============================================================================================= @@ -1151,8 +1192,8 @@ mod tests { let mut rule = ctx_rule("au-1"); rule.graph.parse_mut().find = r"FAILED (\d+) tests in (\S+)".into(); rule.graph.cond = Some(CondStep { finds: Finds::Event, ..Default::default() }); - rule.graph.action.message = "Fix the $1 failing tests in $2".into(); - rule.graph.action.substitute = true; + rule.graph.action_mut().message = "Fix the $1 failing tests in $2".into(); + rule.graph.action_mut().substitute = true; fake.say("pc-1", "FAILED 17 tests in a.ts"); let report = evaluate_once(&engine, host.as_ref(), &rule, "tm-1", 1_000); @@ -1174,8 +1215,8 @@ mod tests { let mut rule = ctx_rule("au-1"); rule.graph.parse_mut().find = r"FAILED (\d+)".into(); rule.graph.cond = Some(CondStep { finds: Finds::Event, ..Default::default() }); - rule.graph.action.message = "Fix $3".into(); - rule.graph.action.substitute = true; + rule.graph.action_mut().message = "Fix $3".into(); + rule.graph.action_mut().substitute = true; fake.say("pc-1", "FAILED 17"); let report = evaluate_once(&engine, host.as_ref(), &rule, "tm-1", 1_000); diff --git a/src-tauri/src/automation_engine/eval.rs b/src-tauri/src/automation_engine/eval.rs index 8d88d979..c87f969f 100644 --- a/src-tauri/src/automation_engine/eval.rs +++ b/src-tauri/src/automation_engine/eval.rs @@ -101,7 +101,12 @@ pub enum ReadDepth { /// the leaf once per pair before calling it and never resolves inside — a function that silently /// accepts either id space is how the next call site gets it wrong. Plan §7.4. pub trait ScreenSource { - fn tail(&self, process_id: &str, depth: ReadDepth) -> Option; + /// `skip_typed_line` is the rule's `monitor.skip_typed_line`, carried here rather than folded + /// into `ReadDepth` because it is not a depth: `depth_for` decides HOW FAR BACK to read, and + /// this decides whether one line of what it finds counts as output at all. Widening the depth + /// enum would put both answers in one table and double its rows for a dimension that does not + /// interact with either of the other two. + fn tail(&self, process_id: &str, depth: ReadDepth, skip_typed_line: bool) -> Option; } // --------------------------------------------------------------------------------------------- @@ -716,7 +721,11 @@ pub fn evaluate( process_id: &str, now_ms: i64, ) -> Option { - evaluate_text(steps, re, echoes, prev, &|d| src.tail(process_id, d), now_ms) + // The ONE place a `ScreenSource` becomes a reader, so the rule's opt-out is applied here rather + // than inside `evaluate_text`: a caller that supplies its own reader is by definition holding + // text that came from somewhere else, and a second gate there could only disagree with this one. + let skip_typed_line = steps.monitor.skip_typed_line; + evaluate_text(steps, re, echoes, prev, &|d| src.tail(process_id, d, skip_typed_line), now_ms) } /// `evaluate` over an already-resolved reader, for a caller that has the text by another route. @@ -868,7 +877,7 @@ mod tests { AutomationGraph { layout: None, timer: None, - monitor: Some(MonitorStep { read: ReadMode::NewOutput, cadence: Cadence::OnOutput, every_ms: 0 }), + monitor: Some(MonitorStep { read: ReadMode::NewOutput, cadence: Cadence::OnOutput, every_ms: 0, skip_typed_line: false }), parse: Some(ParseStep { preset: ParsePreset::Custom, literal: None, @@ -876,13 +885,14 @@ mod tests { keep: Keep::Brackets, }), cond: Some(CondStep { finds, op, threshold, ..Default::default() }), - action: ActionStep { + action: Some(ActionStep { message: "prepare to do context-hand-off".to_string(), send_to: SendTo::Matched, submit: true, cli_type: "default".to_string(), substitute: false, - }, + }), + webhook: None, } } @@ -923,6 +933,12 @@ mod tests { fn feed(&self, line: &str) { self.parser.borrow_mut().process(format!("{}\r\n", line).as_bytes()); } + /// Text left sitting under the cursor: a command typed and NOT submitted. The only + /// difference from `feed` is the missing newline, which is the whole of what separates a + /// keystroke echo from output as far as any of this can tell. + fn type_without_enter(&self, text: &str) { + self.parser.borrow_mut().process(text.as_bytes()); + } fn depths(&self) -> Vec { self.asked.borrow().clone() } @@ -932,7 +948,7 @@ mod tests { } impl ScreenSource for VtSource { - fn tail(&self, _process_id: &str, depth: ReadDepth) -> Option { + fn tail(&self, _process_id: &str, depth: ReadDepth, skip_typed_line: bool) -> Option { self.asked.borrow_mut().push(depth); let mut parser = self.parser.borrow_mut(); let screen = parser.screen_mut(); @@ -940,7 +956,10 @@ mod tests { ReadDepth::Window(n) => n, ReadDepth::VisibleScreen => screen.size().0 as usize, }; - Some(crate::state::render_tail_lines(screen, max)) + // Passed THROUGH, not swallowed: the fake exists to resolve a depth exactly as + // `AppState` does, and a fake that ignored the flag would let `evaluate` stop + // forwarding it with every test here still green. + Some(crate::state::render_tail_lines(screen, max, skip_typed_line)) } } @@ -1715,7 +1734,7 @@ mod tests { "check 5 must re-arm: `FAILED` has left the screen even though scrollback still holds it" ); // The proof that this was a DEPTH decision and not an empty terminal. - let deep = src.tail("pc-1", ReadDepth::Window(200)).unwrap(); + let deep = src.tail("pc-1", ReadDepth::Window(200), false).unwrap(); assert!(deep.contains("FAILED 3 test"), "scrollback must still hold the line"); // 6 — nothing matching. assert_eq!(step(&src, &mut state, 6), Decision::Checked); @@ -1894,7 +1913,7 @@ mod tests { let guarded = if entry == "evaluate" { evaluate(ins(&g), &r, &needles, fired.next, &src, "pc-1", 2).unwrap() } else { - evaluate_text(ins(&g), &r, &needles, fired.next, &|d| src.tail("pc-1", d), 2).unwrap() + evaluate_text(ins(&g), &r, &needles, fired.next, &|d| src.tail("pc-1", d, false), 2).unwrap() }; assert_eq!( guarded.decision, @@ -1908,7 +1927,7 @@ mod tests { let unguarded = if entry == "evaluate" { evaluate(ins(&g), &r, NO_ECHOES, fired.next, &src, "pc-1", 2).unwrap() } else { - evaluate_text(ins(&g), &r, NO_ECHOES, fired.next, &|d| src.tail("pc-1", d), 2).unwrap() + evaluate_text(ins(&g), &r, NO_ECHOES, fired.next, &|d| src.tail("pc-1", d, false), 2).unwrap() }; assert_eq!(unguarded.decision, Decision::Held, "{}: premise", entry); } @@ -2035,7 +2054,7 @@ mod tests { // The line is still well inside the 200-line window. assert!( - src.tail("pc-1", ReadDepth::Window(200)).unwrap().contains("FAILED 3 test"), + src.tail("pc-1", ReadDepth::Window(200), false).unwrap().contains("FAILED 3 test"), "premise: the match is still in scrollback" ); @@ -2053,6 +2072,41 @@ mod tests { ); } + /// `monitor.skip_typed_line` reaches the read — the whole point of the field. + /// + /// This is the DESTINATION check. The rendering half is pinned in `state.rs`'s own + /// `tail_read_tests`, and every one of those would stay green if `evaluate` quietly stopped + /// forwarding the flag: the failure of a plumbing bug is a feature that does nothing, which is + /// invisible from either end on its own. Driving a real `vt100::Parser` is what makes "the + /// command is only typed, not submitted" a fact about a terminal rather than about a fixture. + #[test] + fn a_rule_that_skips_the_typed_line_does_not_read_it() { + let src = VtSource::new(6, 40); + let mut g = failed_rule(); + let r = re(&g.parse_ref().find); + src.feed("running tests"); + src.type_without_enter("$ grep 'FAILED 3 tests' log.txt"); + + // Off — the reported bug, and the premise for the other half: the words ARE on the screen, + // and nothing about them says the user has not pressed Enter yet. + let read = evaluate(ins(&g), &r, NO_ECHOES, ArmState::Unseen, &src, "pc-1", 1).unwrap(); + assert_eq!(read.outcome, Outcome::Presence(true), "premise: it matches what was typed"); + + g.monitor_mut().skip_typed_line = true; + let skipped = evaluate(ins(&g), &r, NO_ECHOES, ArmState::Unseen, &src, "pc-1", 1).unwrap(); + assert_eq!( + skipped.outcome, + Outcome::Presence(false), + "the rule opted out and the typed line was read anyway" + ); + + // And the opt-out costs it nothing once the command is actually submitted. + src.feed(""); + src.feed("FAILED 3 tests in 2 files"); + let submitted = evaluate(ins(&g), &r, NO_ECHOES, ArmState::Unseen, &src, "pc-1", 2).unwrap(); + assert_eq!(submitted.outcome, Outcome::Presence(true), "real output must still match"); + } + /// A terminal that is not live yields no evaluation, no log line and an untouched arm state. #[test] fn a_dormant_terminal_is_skipped_rather_than_re_armed() { diff --git a/src-tauri/src/automation_engine/host.rs b/src-tauri/src/automation_engine/host.rs index bdf42921..b7ee1a0e 100644 --- a/src-tauri/src/automation_engine/host.rs +++ b/src-tauri/src/automation_engine/host.rs @@ -43,7 +43,7 @@ pub trait EngineHost: Send + Sync { fn live_processes(&self) -> Vec; /// Matchable text for one terminal — the `ScreenSource` port, by another name. - fn tail(&self, pc: &str, depth: ReadDepth) -> Option; + fn tail(&self, pc: &str, depth: ReadDepth, skip_typed_line: bool) -> Option; /// One terminal-bound write — the `TerminalWriter` port, by another name. fn write(&self, pc: &str, bytes: &[u8]) -> Result<(), String>; @@ -85,8 +85,8 @@ pub trait EngineHost: Send + Sync { pub struct HostPort<'a>(pub &'a dyn EngineHost); impl ScreenSource for HostPort<'_> { - fn tail(&self, process_id: &str, depth: ReadDepth) -> Option { - self.0.tail(process_id, depth) + fn tail(&self, process_id: &str, depth: ReadDepth, skip_typed_line: bool) -> Option { + self.0.tail(process_id, depth, skip_typed_line) } } diff --git a/src-tauri/src/automation_engine/loops.rs b/src-tauri/src/automation_engine/loops.rs index 88a06ee0..992b9e06 100644 --- a/src-tauri/src/automation_engine/loops.rs +++ b/src-tauri/src/automation_engine/loops.rs @@ -11,9 +11,9 @@ //! milestones that contain the entire engine. use std::collections::{BTreeSet, HashMap, HashSet}; -use std::sync::atomic::Ordering; #[cfg(test)] use std::sync::atomic::AtomicUsize; +use std::sync::atomic::Ordering; use std::sync::Arc; use std::time::Duration; @@ -24,7 +24,9 @@ use crate::automation::targeting::watched_set; use crate::automation_engine::due::{ due_now, select_due, settled_processes, BASE_TICK_MS, MAX_EVALS_PER_TICK, TARGETING_TICK_MS, }; -use crate::automation_engine::eval::{self, ArmState, Captures, Decision, Evaluation, Outcome, Read}; +use crate::automation_engine::eval::{ + self, ArmState, Captures, Decision, Evaluation, Outcome, Read, +}; use crate::automation_engine::host::{EngineHost, HostPort}; use crate::automation_engine::schedule; use crate::automation_engine::subst; @@ -74,7 +76,10 @@ pub async fn run_tap( // so is one extra evaluation each; the cost of dropping it is a missed match with no // symptom. `Lagged` is exactly why the tap carries a signal instead of the bytes. Ok(Err(RecvError::Lagged(n))) => { - log::warn!("automations: tap lagged {} messages, marking every terminal dirty", n); + log::warn!( + "automations: tap lagged {} messages, marking every terminal dirty", + n + ); for pc in host.live_processes() { engine.runtime.mark_dirty(&pc); } @@ -84,6 +89,435 @@ pub async fn run_tap( } } +#[cfg(test)] +mod task8_tests { + use super::*; + use crate::automation_engine::test_host::{ + ctx_rule, + rig_with_rule_bypassing_the_enable_gate, + strip_comments, + wire_bypassing_the_enable_gate, + }; + use crate::automation_store::{LogOrder, LogScope, WebhookProvider, WebhookStep}; + use std::io::{Read, Write}; + use std::net::{TcpListener, TcpStream}; + use std::sync::mpsc::{self, Receiver, Sender}; + + fn pending( + engine: &Arc, + host: &Arc, + prev: ArmState, + at_ms: i64, + ) -> PendingSend { + let rule = engine + .snapshot_live() + .into_iter() + .next() + .expect("live rule"); + engine + .runtime + .set_arm(&rule.rule.id, "tm-1", ArmState::Fired { at_ms }); + PendingSend { + pair: Pair { + rule, + tm: "tm-1".into(), + pc: "pc-1".into(), + }, + prev, + label: host.label_for("tm-1"), + at_ms, + captures: None, + } + } + + fn webhook_endpoint() -> (String, Receiver<()>) { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind loopback webhook listener"); + let url = format!( + "http://{}", + listener.local_addr().expect("listener address") + ); + let (sent, received) = mpsc::channel(); + std::thread::spawn(move || { + let (mut stream, _) = listener.accept().expect("accept webhook request"); + read_webhook_request(&mut stream); + stream + .write_all( + b"HTTP/1.1 204 No Content\r\nContent-Length: 0\r\nConnection: close\r\n\r\n", + ) + .expect("reply"); + sent.send(()).expect("record webhook request"); + }); + (url, received) + } + + /// A one-request endpoint that KEEPS the request bytes. + /// + /// `webhook_endpoint` only reports that something arrived, which is enough for the delivery + /// tests and is exactly not enough for a substitution one: a rule that posts its template + /// verbatim arrives just as reliably as one that posts the resolved body. The bytes are the + /// only place the difference exists. + fn capturing_webhook_endpoint() -> (String, Receiver>) { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind loopback webhook listener"); + let url = format!( + "http://{}", + listener.local_addr().expect("listener address") + ); + let (sent, received) = mpsc::channel(); + std::thread::spawn(move || { + let (mut stream, _) = listener.accept().expect("accept webhook request"); + stream + .set_read_timeout(Some(Duration::from_secs(3))) + .expect("read timeout"); + let mut buffer = [0_u8; 4096]; + let read = stream.read(&mut buffer).expect("read request"); + stream + .write_all( + b"HTTP/1.1 204 No Content +Content-Length: 0 +Connection: close + +", + ) + .expect("reply"); + sent.send(buffer[..read].to_vec()).expect("record webhook request"); + }); + (url, received) + } + + fn held_webhook_endpoint() -> (String, Sender<()>, Receiver<()>) { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind loopback webhook listener"); + let url = format!( + "http://{}", + listener.local_addr().expect("listener address") + ); + let (release_sent, release_received) = mpsc::channel(); + let (arrived_sent, arrived_received) = mpsc::channel(); + std::thread::spawn(move || { + let (mut stream, _) = listener.accept().expect("accept webhook request"); + read_webhook_request(&mut stream); + arrived_sent.send(()).expect("record webhook arrival"); + release_received.recv().expect("release webhook response"); + stream + .write_all( + b"HTTP/1.1 204 No Content\r\nContent-Length: 0\r\nConnection: close\r\n\r\n", + ) + .expect("reply"); + }); + (url, release_sent, arrived_received) + } + + fn read_webhook_request(stream: &mut TcpStream) { + stream + .set_read_timeout(Some(Duration::from_secs(3))) + .expect("read timeout"); + let mut buffer = [0_u8; 4096]; + assert_ne!(stream.read(&mut buffer).expect("read request"), 0); + } + + fn add_discord_webhook(graph: &mut crate::automation_store::AutomationGraph, url: String) { + graph.webhook = Some(WebhookStep { + provider: WebhookProvider::Discord, + url, + body: "webhook body".into(), + substitute: false, + }); + } + + // ============================================================================================= + // Task 8 — one crossing, two destinations + // ============================================================================================= + + /// Normal live-terminal scenario: the terminal stays live after the crossing is decided, but + /// its malformed webhook endpoint fails. That failure must not suppress the terminal delivery. + #[tokio::test] + async fn a_failed_webhook_leaves_the_terminal_send_alone() { + let (engine, fake, host) = rig_with_rule_bypassing_the_enable_gate(|graph| { + // reqwest rejects this while building the request; it cannot leave this machine. + add_discord_webhook(graph, "not a valid URL".into()); + }); + let send = pending(&engine, &host, ArmState::armed(), 4_000); + + run_crossing(engine.clone(), host.clone(), send).await; + + assert!( + fake.written() + .iter() + .any(|write| write.contains("prepare to do context-hand-off")), + "the terminal destination was suppressed by a failed webhook: {:?}", + fake.written() + ); + let rows = fake + .store + .load_automation_log(&LogScope::All, LogOrder::Asc, 10) + .unwrap(); + assert_eq!( + rows.len(), + 2, + "each destination writes its own outcome: {rows:?}" + ); + assert!(rows + .iter() + .any(|row| row.kind == LogKind::Sent && row.terminal_id.as_deref() == Some("tm-1"))); + assert!(rows + .iter() + .any(|row| row.kind == LogKind::Failed && row.terminal_id.is_none())); + } + + /// Already-decided / terminal-closed scenario from the scope note: the leaf disappears after + /// the crossing exists, so the terminal fails, but the webhook still sends once and retires the + /// runs-once rule rather than rolling the arm back for a repeat. + #[tokio::test] + async fn a_failed_terminal_send_does_not_let_the_webhook_repeat() { + let (url, requested) = webhook_endpoint(); + let mut rule = ctx_rule("au-1"); + rule.runs_once = true; + add_discord_webhook(&mut rule.graph, url); + let (engine, fake, host) = wire_bypassing_the_enable_gate(vec![rule]); + let send = pending(&engine, &host, ArmState::armed(), 4_000); + fake.close("tm-1"); + + run_crossing(engine.clone(), host.clone(), send).await; + + requested + .recv_timeout(Duration::from_secs(3)) + .expect("the webhook was sent after the terminal closed"); + assert!( + !engine.is_live("au-1"), + "the successful webhook completed the crossing once" + ); + let rows = fake + .store + .load_automation_log(&LogScope::All, LogOrder::Asc, 10) + .unwrap(); + assert!(rows + .iter() + .any(|row| row.kind == LogKind::Failed && row.terminal_id.as_deref() == Some("tm-1"))); + assert!(rows + .iter() + .any(|row| row.kind == LogKind::Sent && row.terminal_id.is_none())); + } + + /// Normal live-terminal scenario: both destinations succeed for one already-decided crossing, + /// so its fire history increments once rather than once per destination. + #[tokio::test] + async fn a_crossing_records_one_fire_however_many_destinations_it_had() { + let (url, requested) = webhook_endpoint(); + let (engine, _fake, host) = + rig_with_rule_bypassing_the_enable_gate(|graph| add_discord_webhook(graph, url)); + let send = pending(&engine, &host, ArmState::armed(), 4_000); + + run_crossing(engine.clone(), host.clone(), send).await; + + requested + .recv_timeout(Duration::from_secs(3)) + .expect("webhook request"); + assert_eq!( + engine.runtime.fire_record("au-1", "tm-1"), + Some((1, 4_000)), + "two destinations are one crossing, not two fires" + ); + } + + /// Normal live-terminal scenario: the terminal has finished its 500 ms delivery while the + /// webhook deliberately waits for its local response. Completion must wait for that response. + #[tokio::test] + async fn completion_waits_for_every_destination_not_the_first() { + let (url, release, arrived) = held_webhook_endpoint(); + let mut rule = ctx_rule("au-1"); + rule.runs_once = true; + add_discord_webhook(&mut rule.graph, url); + let (engine, fake, host) = wire_bypassing_the_enable_gate(vec![rule]); + let send = pending(&engine, &host, ArmState::armed(), 4_000); + let task = tokio::spawn(run_crossing(engine.clone(), host.clone(), send)); + + tokio::time::sleep(Duration::from_millis(100)).await; + arrived + .try_recv() + .expect("webhook request reached the held listener"); + tokio::time::sleep(Duration::from_millis(650)).await; + assert!( + fake.written() + .iter() + .any(|write| write.contains("prepare to do context-hand-off")), + "the terminal destination did not complete" + ); + assert!( + engine.is_live("au-1"), + "the first completed destination retired the rule early" + ); + assert!( + !task.is_finished(), + "the crossing returned before the held webhook did" + ); + + release.send(()).expect("release webhook response"); + task.await.expect("crossing task"); + assert!( + !engine.is_live("au-1"), + "completion did not follow both destination outcomes" + ); + } + + /// Normal live-terminal scenario: this is the strong two-row oracle. Both rows must name the + /// same rule and decision timestamp, while their terminal identity, provider identity, kind, + /// and observed delivery outcomes distinguish a terminal send from a webhook send. + #[tokio::test] + async fn one_crossing_with_two_destinations_writes_one_row_each() { + let (url, requested) = webhook_endpoint(); + let (engine, fake, host) = + rig_with_rule_bypassing_the_enable_gate(|graph| add_discord_webhook(graph, url)); + let send = pending(&engine, &host, ArmState::armed(), 4_242); + + run_crossing(engine.clone(), host.clone(), send).await; + + requested + .recv_timeout(Duration::from_secs(3)) + .expect("webhook request"); + assert!( + fake.written() + .iter() + .any(|write| write.contains("prepare to do context-hand-off")), + "the terminal delivery did not occur" + ); + let rows = fake + .store + .load_automation_log(&LogScope::All, LogOrder::Asc, 10) + .unwrap(); + assert_eq!(rows.len(), 2, "not a bare count: inspect both rows below"); + let terminal = rows + .iter() + .find(|row| row.terminal_id.as_deref() == Some("tm-1")) + .expect("terminal destination row"); + assert_eq!(terminal.rule_id, "au-1"); + assert_eq!(terminal.at, 4_242); + assert_eq!(terminal.kind, LogKind::Sent); + assert_eq!(terminal.terminal_name.as_deref(), Some("codex · core")); + assert_eq!(terminal.detail, "sent to codex · core"); + + let webhook = rows + .iter() + .find(|row| row.terminal_id.is_none()) + .expect("webhook destination row"); + assert_eq!(webhook.rule_id, "au-1"); + assert_eq!(webhook.at, 4_242); + assert_eq!(webhook.kind, LogKind::Sent); + assert_eq!(webhook.terminal_name, None); + assert_eq!(webhook.detail, "webhook sent via Discord"); + } + + /// Source-derived rather than behavioural: the dispatch code has no clock of its own, and the + /// webhook sibling never takes `send_lock`. This test exercises no terminal scenario. + #[test] + fn the_webhook_path_adds_no_new_clock() { + let source = strip_comments(include_str!("loops.rs")); + let crossing_start = source + .rfind("async fn run_crossing(") + .expect("crossing function"); + let webhook_start = source + .rfind("async fn run_webhook(") + .expect("webhook function"); + let terminal_start = source + .rfind("async fn run_send(") + .expect("terminal function"); + let completion_start = source + .rfind("fn complete_crossing(") + .expect("completion function"); + let crossing = &source[crossing_start..terminal_start]; + let webhook = &source[webhook_start..completion_start]; + assert!( + crossing.contains("tokio::join!"), + "destinations must be aggregated together" + ); + for body in [crossing, webhook] { + assert!( + !body.contains("tokio::time::interval"), + "a webhook path added an interval" + ); + assert!( + !body.contains("tokio::time::sleep"), + "a webhook path added a sleep" + ); + } + assert!( + !webhook.contains("send_lock"), + "a webhook must not queue behind a terminal send" + ); + assert!(crossing.contains("run_send") && crossing.contains("run_webhook")); + } + + /// **The webhook destination posts the RESOLVED body**, the exact twin of the test above. + /// + /// Reported from a live build: *"on Discord I got the $0, not the matched value"*. The + /// substitution in `run_webhook` was correct, and nothing covered it — every webhook test until + /// now asserted only that a request ARRIVED, which a rule posting its template verbatim does + /// just as reliably. `capturing_webhook_endpoint` exists so the assertion can be about the + /// bytes, where the difference actually lives. + /// + /// Driven through `run_crossing` with a hand-built `Captures` rather than through a tick, + /// because the two neighbouring webhook tests do: reqwest is real I/O and the tick-driven + /// tests run on a paused clock. + #[tokio::test] + async fn a_crossing_posts_the_resolved_webhook_body() { + let (url, posted) = capturing_webhook_endpoint(); + let (engine, _fake, host) = rig_with_rule_bypassing_the_enable_gate(|graph| { + add_discord_webhook(graph, url); + let webhook = graph.webhook.as_mut().expect("the webhook just added"); + webhook.body = "Fix the $1 failing tests in $2".into(); + webhook.substitute = true; + }); + let mut send = pending(&engine, &host, ArmState::armed(), 4_000); + send.captures = Some(Captures { + groups: vec![ + Some("FAILED 17 tests in a.ts".into()), + Some("17".into()), + Some("a.ts".into()), + ], + named: Default::default(), + }); + + run_crossing(engine.clone(), host.clone(), send).await; + + let request = posted + .recv_timeout(Duration::from_secs(3)) + .expect("the webhook was never posted"); + let text = String::from_utf8_lossy(&request); + assert!( + text.contains("Fix the 17 failing tests in a.ts"), + "the resolved body never reached the wire: {text}" + ); + // The complaint in its own words: the token itself must not survive the send. + assert!(!text.contains("$1"), "a raw token was posted: {text}"); + } + + /// The other half of the pair, and the reason the flag is worth having: with substitution off + /// the body is posted EXACTLY as typed. Asserted so that "resolved" above cannot be satisfied + /// by a sender that always substitutes — a webhook body is sometimes JSON a user wrote by + /// hand, and `$` is not always a token. + #[tokio::test] + async fn a_webhook_that_opted_out_posts_its_body_verbatim() { + let (url, posted) = capturing_webhook_endpoint(); + let (engine, _fake, host) = rig_with_rule_bypassing_the_enable_gate(|graph| { + add_discord_webhook(graph, url); + let webhook = graph.webhook.as_mut().expect("the webhook just added"); + webhook.body = "Fix the $1 failing tests in $2".into(); + webhook.substitute = false; + }); + let mut send = pending(&engine, &host, ArmState::armed(), 4_000); + send.captures = Some(Captures { + groups: vec![Some("FAILED 17 tests in a.ts".into()), Some("17".into())], + named: Default::default(), + }); + + run_crossing(engine.clone(), host.clone(), send).await; + + let request = posted + .recv_timeout(Duration::from_secs(3)) + .expect("the webhook was never posted"); + let text = String::from_utf8_lossy(&request); + assert!(text.contains("$1"), "the opted-out body was rewritten: {text}"); + } +} + // ================================================================================================= // The evaluator (§2.3) // ================================================================================================= @@ -215,7 +649,11 @@ pub async fn evaluator_step( // the pair's *"Waiting to send"* pill sits on a countdown that reached zero and stopped, // until something unrelated repaints it. Marked and not emitted: one drain point, one rate // limit, and `evaluate_tick` below is that point. - if engine.runtime.drop_stale_parked(now_ms, crate::automation_validation::MAX_DELAY_MS) > 0 { + if engine + .runtime + .drop_stale_parked(now_ms, crate::automation_validation::MAX_DELAY_MS) + > 0 + { engine.mark_state_dirty(); } } @@ -255,7 +693,11 @@ pub async fn evaluate_tick( let now_local = schedule::local_now(now_ms); for live in engine.snapshot_live() { // Sorted, so which pairs the cap holds over is a property of the rule and not of hash order. - let mut leaves: Vec = engine.runtime.watched_for(&live.rule.id).into_iter().collect(); + let mut leaves: Vec = engine + .runtime + .watched_for(&live.rule.id) + .into_iter() + .collect(); leaves.sort(); // **§6.3's two rule-level facts, decided BEFORE the leaves.** // @@ -278,11 +720,17 @@ pub async fn evaluate_tick( // crossing as well as on the clock. `schedule_due` is deliberately false for `AfterMatch`, so // it cannot double as the "is this a schedule rule" question: that is what `scheduled` is. let scheduled = match &live.rule.graph.timer { - Some(TimerStep { mode: mode @ TimerMode::DailyAt { .. } }) => Some(mode), + Some(TimerStep { + mode: mode @ TimerMode::DailyAt { .. }, + }) => Some(mode), _ => None, }; let fires_now = scheduled.is_some_and(|mode| { - schedule::schedule_due(mode, engine.runtime.last_fired_day(&live.rule.id), now_local) + schedule::schedule_due( + mode, + engine.runtime.last_fired_day(&live.rule.id), + now_local, + ) }); for tm in leaves { // The ONE tm -> pc conversion. `None` is dormant (§4.5), not dead: no evaluation, no log @@ -332,21 +780,22 @@ pub async fn evaluate_tick( // message it is about to type. if let Some(parked) = engine.runtime.take_parked_due(&live.rule.id, &tm, now_ms) { admit( - engine, - host, &mut sends, PendingSend { // **`parked.pc`, never the `pc` this tick just resolved.** The restart // guard in `run_send` compares the leaf's process at lock time against // this field; filled from the drain's own lookup it compares a value // against itself and the whole park is unguarded. - pair: Pair { rule: live.clone(), tm: tm.clone(), pc: parked.pc }, + pair: Pair { + rule: live.clone(), + tm: tm.clone(), + pc: parked.pc, + }, prev: parked.prev, label: parked.label, at_ms: now_ms, captures: parked.captures, }, - now_ms, ); } let seq = engine.runtime.dirty_seq(&pc); @@ -376,11 +825,13 @@ pub async fn evaluate_tick( if scheduled.is_some() { if fires_now { admit( - engine, - host, &mut sends, PendingSend { - pair: Pair { rule: live.clone(), tm: tm.clone(), pc }, + pair: Pair { + rule: live.clone(), + tm: tm.clone(), + pc, + }, // **Read, not assumed.** `prev` is what `run_send`'s three failure paths // roll back to, and a schedule rule has no crossing to roll back to — so // the only correct target is whatever is already there, which makes @@ -402,7 +853,6 @@ pub async fn evaluate_tick( // template into a live agent. captures: None, }, - now_ms, ); } continue; @@ -432,7 +882,11 @@ pub async fn evaluate_tick( engine.runtime.last_eval(&live.rule.id, &tm), now_ms, ) { - due.push(Pair { rule: live.clone(), tm, pc }); + due.push(Pair { + rule: live.clone(), + tm, + pc, + }); } else if monitor.cadence == Cadence::OnOutput { // No `seq.is_some()` here, deliberately: a CLEAN process contributes no due pair, so // it never reaches `due_pcs` and `settled_processes` can never name it — the extra @@ -456,7 +910,9 @@ pub async fn evaluate_tick( let Some(TimerMode::DailyAt { minute_of_day, .. }) = scheduled else { unreachable!("fires_now requires a daily schedule"); }; - engine.runtime.set_last_fired_day(&live.rule.id, now_local.day_ordinal, *minute_of_day); + engine + .runtime + .set_last_fired_day(&live.rule.id, now_local.day_ordinal, *minute_of_day); } } @@ -472,7 +928,7 @@ pub async fn evaluate_tick( for i in &picked { match evaluate_pair(engine, host, &due[*i], now_ms) { - Evaluated::Read(Some(send)) => admit(engine, host, &mut sends, send, now_ms), + Evaluated::Read(Some(send)) => admit(&mut sends, send), // Read, decided, nothing to send: this pair has consumed the output and may spend it. Evaluated::Read(None) => {} // **The third door.** `settled_processes`'s enumeration named two and this was neither: @@ -497,9 +953,16 @@ pub async fn evaluate_tick( // sends in one tick would freeze evaluation for two seconds. Serialisation is unaffected: it was // never the tick that provided it, it was the per-terminal lock. for send in sends { + // A crossing owns its destinations and all of their shared bookkeeping. In particular, a + // webhook-only rule has no terminal destination: never manufacture an ActionStep merely to + // route it through `run_send`, because an empty action can submit a bare Enter. + if send.pair.rule.rule.graph.action.is_none() && send.pair.rule.rule.graph.webhook.is_none() + { + continue; + } let engine = engine.clone(); let host = host.clone(); - tokio::spawn(async move { run_send(engine, host, send).await }); + tokio::spawn(async move { run_crossing(engine, host, send).await }); } if engine.take_state_emit(now_ms) { @@ -509,59 +972,13 @@ pub async fn evaluate_tick( next_cursor } -/// Put one decided send on this tick's dispatch list, if R6 lets it through. -/// -/// **R6 is per RULE — not per pair, and not per TICK.** A `runs_once` rule watching three terminals -/// crosses on all three, and the send lock is per LEAF, so three tasks take three different locks -/// and three messages go out on a rule the user asked to run once. +/// Put one decided crossing on this tick's dispatch list. /// -/// The claim is taken HERE, where the crossing is decided. The first version of this scanned the -/// current tick's `sends` vector, which covers the three-in-one-tick case and nothing else: two -/// terminals crossing on consecutive ticks are two separate vectors, and the only cross-tick guard -/// was `is_live`, which does not go false until `complete_rule` runs — after `deliver` returns, two -/// ticks later. The arm states advance either way; only the send is dropped, so a pair that did not -/// send stays `Fired` and never sends. -/// -/// **It is a function because there are now two routes onto that list**, and a gate written at one -/// caller is a gate the next caller opts out of. §6.2's parked sends are the second route, and they -/// need it more than the first: a `runs_once` rule with a delay parks on every terminal that -/// crosses *during* the wait — the arm machine cannot stop that, because those are different pairs — -/// and all of them come ripe on the same tick, where without this they would be three sends. -fn admit( - engine: &Arc, - host: &Arc, - sends: &mut Vec, - send: PendingSend, - now_ms: i64, -) { - let rule_id = &send.pair.rule.rule.id; - if !send.pair.rule.rule.runs_once || engine.runtime.claim_once(rule_id) { - sends.push(send); - return; - } - // **A dropped crossing that says nothing is a crossing the user cannot account for.** The arm - // state advanced at decide time and the parked entry was taken out of the map by - // `take_parked_due`, so this pair is finished either way — and on the delay route it had been - // visibly *"Waiting to send"* for up to ten minutes first. Without a row the only trace of it is - // a countdown that stopped. - // - // `Held` and not `Failed`, for the seeding row's reason: nothing went wrong. The rule was asked - // and the rule declined, which is also what keeps it out of the verbose gate. It is bounded by - // the claim itself — one row per losing pair per crossing, and a claimed `runs_once` rule - // leaves the live set as soon as its send lands. - // - // Written HERE rather than at the parked drain that found it, because `admit` is the one gate - // all three routes go through and a row written at one caller is a row the next caller opts out - // of — the same reasoning that put the claim itself in this function. - append( - host, - rule_id, - Some(&send.pair.tm), - send.label.clone(), - LogKind::Held, - "not sent — this rule runs once, and another terminal had already claimed its one send", - now_ms, - ); +/// Both immediate and parked crossings use this one dispatch seam. Admission, including the +/// runs-once claim, belongs to `run_crossing`: it is crossing-wide bookkeeping rather than a +/// property of either destination. +fn admit(sends: &mut Vec, send: PendingSend) { + sends.push(send); } /// What one pair's evaluation leaves for the tick to do. @@ -615,15 +1032,9 @@ pub fn evaluate_pair( return Evaluated::Unread; }; - let Some(ev): Option = eval::evaluate( - steps, - re, - &echoes, - prev, - &port, - &pair.pc, - now_ms, - ) else { + let Some(ev): Option = + eval::evaluate(steps, re, &echoes, prev, &port, &pair.pc, now_ms) + else { // `host.tail` found no parser for this process — it closed between this tick's leaf // resolution and the read. §4.5: no evaluation, no row, arm state untouched. `set_last_eval` // is deliberately not reached either, so the pair is due again immediately. @@ -644,12 +1055,22 @@ pub fn evaluate_pair( } let repeat = engine.runtime.last_decision(&rule.id, &pair.tm) == Some(ev.decision); - engine.runtime.set_last_decision(&rule.id, &pair.tm, ev.decision); + engine + .runtime + .set_last_decision(&rule.id, &pair.tm, ev.decision); if !ev.decision.sends() { // Live by construction: `evaluate_pair` only runs for a pair whose leaf just resolved. let name = host.label_for(&pair.tm); - append(host, &rule.id, Some(&pair.tm), name, kind_for(&ev, repeat), &ev.detail, now_ms); + append( + host, + &rule.id, + Some(&pair.tm), + name, + kind_for(&ev, repeat), + &ev.detail, + now_ms, + ); return Evaluated::Read(None); } @@ -660,7 +1081,10 @@ pub fn evaluate_pair( // it. `Read(None)` and not `Unread`, because this pair genuinely READ the terminal's output — // that read is how it found the match — so the dirty flag is spent exactly as it would have // been by a send. - if let Some(TimerStep { mode: TimerMode::AfterMatch { delay_ms } }) = &rule.graph.timer { + if let Some(TimerStep { + mode: TimerMode::AfterMatch { delay_ms }, + }) = &rule.graph.timer + { engine.runtime.park( &rule.id, &pair.tm, @@ -726,21 +1150,156 @@ fn kind_for(ev: &Evaluation, repeat: bool) -> LogKind { } // ================================================================================================= -// The send (§2.5, §2.6) +// The crossing and its destinations (§2.5, §2.6, A8) // ================================================================================================= -/// Take the terminal's queue, re-check it is still there, write, and record what happened. +/// The result of one destination, deliberately separate from the crossing's shared state. +enum DestinationOutcome { + Sent, + Failed(String), + Stopped, +} + +/// Dispatch both destinations for one already-decided crossing, then perform its bookkeeping once. /// -/// **Every failure path rolls the arm state back and writes exactly one log line** — the queue timed -/// out, the terminal closed between the decision and the write, the write itself failed. Never left -/// `Fired`, because a crossing that produced no message must still be able to fire. -pub async fn run_send( - engine: Arc, - host: Arc, - send: PendingSend, -) { +/// A terminal delivery and a webhook are independent side effects: one failure must not prevent the +/// other from running. Their common effects — the fire history, runs-once completion, and rollback — +/// are therefore deliberately below the join, where there is one answer for the crossing rather than +/// one answer per destination. +async fn run_crossing(engine: Arc, host: Arc, send: PendingSend) { + let rule = &send.pair.rule.rule; + if engine.stopping.load(Ordering::Relaxed) { + rollback_crossing(&engine, &send); + return; + } + + // R6 is per crossing, not per terminal destination. This task begins at the one shared + // boundary before the concurrent sends, so a second crossing cannot send either destination + // after the first one has claimed the rule. + if rule.runs_once && !engine.runtime.claim_once(&rule.id) { + append( + &host, + &rule.id, + Some(&send.pair.tm), + send.label.clone(), + LogKind::Held, + "not sent — this rule runs once, and another terminal had already claimed its one send", + send.at_ms, + ); + return; + } + + let has_action = rule.graph.action.is_some(); + let has_webhook = rule.graph.webhook.is_some(); + let (terminal, webhook) = tokio::join!( + async { + if has_action { + Some(run_send(&engine, &host, &send).await) + } else { + None + } + }, + async { + if has_webhook { + Some(run_webhook(&engine, &send).await) + } else { + None + } + }, + ); + + let outcomes = [terminal, webhook]; + if outcomes + .iter() + .flatten() + .any(|outcome| matches!(outcome, DestinationOutcome::Stopped)) + { + rollback_crossing(&engine, &send); + return; + } + + let mut sent = false; + if let Some(outcome) = outcomes[0].as_ref() { + match outcome { + DestinationOutcome::Sent => { + sent = true; + append( + &host, + &rule.id, + Some(&send.pair.tm), + send.label.clone(), + LogKind::Sent, + &sent_detail(&send), + send.at_ms, + ); + } + DestinationOutcome::Failed(reason) => append( + &host, + &rule.id, + Some(&send.pair.tm), + send.label.clone(), + LogKind::Failed, + reason, + send.at_ms, + ), + DestinationOutcome::Stopped => unreachable!("stopped outcomes returned above"), + } + } + if let Some(outcome) = outcomes[1].as_ref() { + match outcome { + DestinationOutcome::Sent => { + sent = true; + append( + &host, + &rule.id, + None, + None, + LogKind::Sent, + &webhook_sent_detail(&send), + send.at_ms, + ); + } + DestinationOutcome::Failed(reason) => append( + &host, + &rule.id, + None, + None, + LogKind::Failed, + reason, + send.at_ms, + ), + DestinationOutcome::Stopped => unreachable!("stopped outcomes returned above"), + } + } + + if !sent { + rollback_crossing(&engine, &send); + return; + } + + // One crossing records one fire however many destinations completed. A terminal echo/settle + // remains owned by `run_send`, because only terminal bytes can be echoed back into a screen. + engine + .runtime + .record_fire(&rule.id, &send.pair.tm, send.at_ms); + complete_crossing(&engine, &host, &send); + engine.mark_state_dirty(); +} + +/// Take the terminal's queue, re-check it is still there, and write its destination. +/// +/// This function has no crossing bookkeeping: `run_crossing` aggregates its result with the webhook +/// before it records a fire, completes a runs-once rule, or rolls an arm back. +async fn run_send( + engine: &Arc, + host: &Arc, + send: &PendingSend, +) -> DestinationOutcome { let rule = &send.pair.rule.rule; let tm = send.pair.tm.clone(); + let Some(action) = rule.graph.action.as_ref() else { + return DestinationOutcome::Failed("the rule has no terminal destination".into()); + }; // **Before the queue.** §2.6 layer 2 runs for `ECHO_SETTLE_MS` after the WRITE, and the wait for // this terminal's lock is up to `SEND_QUEUE_TIMEOUT_MS` of the distance between the decision and // that write. Started after the lock, this measured only `deliver` — so the second and later @@ -749,21 +1308,20 @@ pub async fn run_send( let began = tokio::time::Instant::now(); let lock = engine.runtime.send_lock(&tm); - let _guard = match tokio::time::timeout( - Duration::from_millis(SEND_QUEUE_TIMEOUT_MS), - lock.lock(), - ) - .await - { - Ok(guard) => guard, - Err(_) => { - return fail(&engine, &host, &send, "another rule was still sending"); - } - }; + let _guard = + match tokio::time::timeout(Duration::from_millis(SEND_QUEUE_TIMEOUT_MS), lock.lock()).await + { + Ok(guard) => guard, + Err(_) => { + return DestinationOutcome::Failed("another rule was still sending".into()); + } + }; // Inside the lock, because the terminal can close between the decision and our turn at the queue. let Some(pc) = host.process_for_leaf(&tm) else { - return fail(&engine, &host, &send, "the terminal closed before the message was sent"); + return DestinationOutcome::Failed( + "the terminal closed before the message was sent".into(), + ); }; // **And it can close AND COME BACK, which resolving by leaf alone cannot see.** `tm-` is durable @@ -781,7 +1339,9 @@ pub async fn run_send( // carries the crossing's process across that wait — built from the drain's own lookup instead, // this comparison would be a value against itself for every delayed rule. if pc != send.pair.pc { - return fail(&engine, &host, &send, "the terminal restarted before the message was sent"); + return DestinationOutcome::Failed( + "the terminal restarted before the message was sent".into(), + ); } // And so can the RULE. The queue wait is up to ten seconds, and a user who disables a rule inside @@ -793,24 +1353,17 @@ pub async fn run_send( // that completed on another terminal, and told the user their rule had been turned off when // nobody had touched it. if !engine.is_live(&rule.id) { - return fail(&engine, &host, &send, "the rule was turned off before the message was sent"); + return DestinationOutcome::Failed( + "the rule was turned off before the message was sent".into(), + ); } // §2.1: checked before the FIRST write and never between the paste and the submit, so a quit // leaves the send either unstarted or complete — there is no half-typed line to reason about. if engine.stopping.load(Ordering::Relaxed) { - // The same rollback as `fail`, minus the row — the app is going down and the store is - // closing. It was the one rollback of three that announced nothing, so a pill caught - // mid-transition stayed on whatever it had last painted. - engine.runtime.restore_arm(&rule.id, &tm, send.prev); - if rule.runs_once { - engine.runtime.release_once(&rule.id); - } - engine.mark_state_dirty(); - return; + return DestinationOutcome::Stopped; } - let action = &rule.graph.action; let body = if action.substitute { match subst::substitute(&action.message, send.captures.as_ref()) { Ok(s) => s, @@ -818,12 +1371,9 @@ pub async fn run_send( // the "unintended content" this whole feature exists to prevent, and a refusal that is // logged is the safe fallback it asks for instead. Err(e) => { - return fail( - &engine, - &host, - &send, - &format!("nothing sent — {e} had no value at the moment it fired"), - ); + return DestinationOutcome::Failed(format!( + "nothing sent — {e} had no value at the moment it fired" + )); } } } else { @@ -835,14 +1385,17 @@ pub async fn run_send( &HostPort(host.as_ref()), &pc, &action.cli_type, - crate::automation::send::SubmitPattern { separator, end_indicator }, + crate::automation::send::SubmitPattern { + separator, + end_indicator, + }, &body, action.submit, ) .await; if let Err(e) = outcome { - return fail(&engine, &host, &send, &format!("the message could not be sent: {}", e)); + return DestinationOutcome::Failed(format!("the message could not be sent: {}", e)); } let at = send.at_ms; @@ -855,14 +1408,57 @@ pub async fn run_send( // still strips it. The needle is `body` — what actually reached the terminal — never // `action.message`: with substitution on, the terminal echoes the RESOLVED text, and a needle // still carrying `$1` would never match it. - engine.runtime.push_echo(&tm, &crate::automation::send::normalise(&body), landed); + engine + .runtime + .push_echo(&tm, &crate::automation::send::normalise(&body), landed); engine.runtime.settle_until(&tm, landed + ECHO_SETTLE_MS); - engine.runtime.record_fire(&rule.id, &tm, at); + DestinationOutcome::Sent +} - let name = send.label.clone(); - append(&host, &rule.id, Some(&tm), name, LogKind::Sent, &sent_detail(&send), at); +/// Post the webhook destination for this crossing. It intentionally never reaches the terminal +/// queue lock: a slow endpoint must not serialise terminal writes, and a terminal failure must not +/// suppress an already-decided webhook. +async fn run_webhook(engine: &Arc, send: &PendingSend) -> DestinationOutcome { + let rule = &send.pair.rule.rule; + let Some(webhook) = rule.graph.webhook.as_ref() else { + return DestinationOutcome::Failed("the rule has no webhook destination".into()); + }; + if engine.stopping.load(Ordering::Relaxed) { + return DestinationOutcome::Stopped; + } + if !engine.is_live(&rule.id) { + return DestinationOutcome::Failed( + "the rule was turned off before the webhook was sent".into(), + ); + } + let body = if webhook.substitute { + match subst::substitute(&webhook.body, send.captures.as_ref()) { + Ok(body) => body, + Err(e) => { + return DestinationOutcome::Failed(format!( + "webhook not sent — {e} had no value at the moment it fired" + )) + } + } + } else { + webhook.body.clone() + }; + match crate::automation_webhook::send_body(webhook, &body).await { + Ok(()) => DestinationOutcome::Sent, + Err(error) => DestinationOutcome::Failed(format!("webhook failed: {error}")), + } +} - // §7.8 — completion is an in-memory event FIRST and a row second, in this same critical section. +/// Complete a successful crossing once, after every destination has returned. +fn complete_crossing( + engine: &Arc, + host: &Arc, + send: &PendingSend, +) { + let rule = &send.pair.rule.rule; + let tm = &send.pair.tm; + let at = send.at_ms; + // §7.8 — completion is an in-memory event FIRST and a row second, after every destination. // `reload` runs from mutating store commands and this is the engine, which is not one: without // the in-memory removal the rule stays live in `Fired`, re-arms the moment its value drops, and // sends a SECOND message in the same session from a row the UI already shows as Completed. @@ -893,7 +1489,7 @@ pub async fn run_send( append( &host, &rule.id, - Some(&tm), + Some(tm), None, LogKind::Failed, "fired, but its completion could not be recorded — it may run again after a restart", @@ -918,7 +1514,6 @@ pub async fn run_send( // the thing that failed, which makes disk stale rather than true. host.emit_changed(vec![rule.id.clone()]); } - engine.mark_state_dirty(); } fn sent_detail(send: &PendingSend) -> String { @@ -928,15 +1523,25 @@ fn sent_detail(send: &PendingSend) -> String { } } -/// One failure: roll the arm state back to exactly where it was, and say so once. -fn fail( - engine: &Arc, - host: &Arc, - send: &PendingSend, - reason: &str, -) { +fn webhook_sent_detail(send: &PendingSend) -> String { + let provider = send + .pair + .rule + .rule + .graph + .webhook + .as_ref() + .expect("a webhook outcome requires a webhook step") + .provider; + format!("webhook sent via {provider:?}") +} + +/// Roll one wholly failed crossing back to exactly the arm state it had when it was decided. +fn rollback_crossing(engine: &Arc, send: &PendingSend) { let rule_id = &send.pair.rule.rule.id; - engine.runtime.restore_arm(rule_id, &send.pair.tm, send.prev); + engine + .runtime + .restore_arm(rule_id, &send.pair.tm, send.prev); // A rollback restores; it never creates. The claim was taken when this crossing was DECIDED, so a // crossing that produced no message must give it back — otherwise one queue timeout retires a // single-run rule that has never sent anything. @@ -944,17 +1549,6 @@ fn fail( engine.runtime.release_once(rule_id); } engine.mark_state_dirty(); - append( - host, - rule_id, - Some(&send.pair.tm), - // The label resolved at DECIDE time. This is the whole reason `PendingSend` carries one: - // `the terminal closed` is written when there is no name left to look up. - send.label.clone(), - LogKind::Failed, - reason, - send.at_ms, - ); } /// Append one row and emit if the store says one is due. @@ -989,7 +1583,11 @@ fn append( match host.store().append(&entry) { Ok(Some(outcome)) if outcome.emit => host.emit_activity(outcome.rule_ids), Ok(_) => {} - Err(e) => log::warn!("automations: could not write a log row for {}: {}", rule_id, e), + Err(e) => log::warn!( + "automations: could not write a log row for {}: {}", + rule_id, + e + ), } } @@ -1007,8 +1605,10 @@ pub async fn run_targeting(engine: Arc, host: Arc>, HashMap>) = - (HashMap::new(), HashMap::new()); + let mut last: ( + HashMap>, + HashMap>, + ) = (HashMap::new(), HashMap::new()); loop { if engine.stopping.load(Ordering::Relaxed) { return; @@ -1017,7 +1617,8 @@ pub async fn run_targeting(engine: Arc, host: Arc pass, Err(e) => { // `unwrap_or_default()` here turned a panicked roster pass into an EMPTY one, which @@ -1075,14 +1676,19 @@ pub fn targeting_tick( let criteria: Vec = rules .iter() .filter(|l| l.rule.target_mode == TargetMode::Rule) - .map(|l| l.rule.criterion) + .flat_map(|l| std::iter::once(l.rule.criterion).chain(l.rule.exclude_criterion)) .collect(); let rows = host.roster(&criteria); // Indexed once, outside the rule loop: the snapshot walk below wants the row for a terminal it // already knows it watches, and scanning the whole roster per rule made that `rules × roster`. - let by_id: HashMap<&str, &crate::automation::roster::RosterRow> = - rows.iter().filter_map(|r| r.terminal_id.as_deref().map(|t| (t, r))).collect(); - let live_leaves: HashSet<&str> = rows.iter().filter_map(|r| r.terminal_id.as_deref()).collect(); + let by_id: HashMap<&str, &crate::automation::roster::RosterRow> = rows + .iter() + .filter_map(|r| r.terminal_id.as_deref().map(|t| (t, r))) + .collect(); + let live_leaves: HashSet<&str> = rows + .iter() + .filter_map(|r| r.terminal_id.as_deref()) + .collect(); let grace_over = crate::automation::roster::grace_elapsed(now_ms, engine.started_at_ms()); let mut missing = HashMap::new(); let mut watched: HashMap> = HashMap::new(); @@ -1095,7 +1701,9 @@ pub fn targeting_tick( // consumer.) let previous: BTreeSet = engine.runtime.watched_for(id).into_iter().collect(); let next = watched_set(&live.rule, &rows, Some(&previous)); - engine.runtime.set_watched(id, next.iter().cloned().collect()); + engine + .runtime + .set_watched(id, next.iter().cloned().collect()); // §2.4: *"keys are cleared when … a terminal leaves the watch set"*. Three of that // sentence's four events were implemented and this one was not. A `Command contains` rule @@ -1116,17 +1724,24 @@ pub fn targeting_tick( let Some(row) = by_id.get(tm.as_str()) else { continue; }; - let label = crate::automation::labels::label_at(&crate::automation::labels::LabelInputs { - display_label: row.display_label.as_deref(), - name: Some(row.name.as_str()), - shell: Some(row.shell.as_str()), - // Writing the snapshot, so the snapshot is not an input to it. - snapshot: None, - }); + let label = + crate::automation::labels::label_at(&crate::automation::labels::LabelInputs { + display_label: row.display_label.as_deref(), + name: Some(row.name.as_str()), + shell: Some(row.shell.as_str()), + // Writing the snapshot, so the snapshot is not an input to it. + snapshot: None, + }); if let Err(e) = - host.store().touch_target(id, tm, label.as_deref(), row.cwd.as_deref(), now_ms) + host.store() + .touch_target(id, tm, label.as_deref(), row.cwd.as_deref(), now_ms) { - log::warn!("automations: could not record {}'s view of {}: {}", id, tm, e); + log::warn!( + "automations: could not record {}'s view of {}: {}", + id, + tm, + e + ); } } @@ -1147,14 +1762,13 @@ pub fn targeting_tick( TargetingPass { missing, watched } } - #[cfg(test)] mod tests { use super::*; // The fake, the canonical rule and the wiring are shared with the dry run's tests so there can // only ever be one of each. - use crate::automation_engine::test_host::*; use crate::automation::roster::RosterRow; + use crate::automation_engine::test_host::*; use crate::automation_store::{AutomationRule, Finds, Keep}; use chrono::{Datelike, Local, NaiveDate, TimeZone, Weekday}; @@ -1178,17 +1792,26 @@ mod tests { // Twenty into a channel of four, before the tap has read any of them: the receiver is // guaranteed to see `Lagged`, which is the case that must not silently drop terminals. for i in 0..20 { - let _ = tx.send(ChannelPayload { id: format!("pc-{}", i % 2 + 1), data: vec![b'x'] }); + let _ = tx.send(ChannelPayload { + id: format!("pc-{}", i % 2 + 1), + data: vec![b'x'], + }); } let tap = tokio::spawn(run_tap(engine.clone(), host.clone(), rx)); tokio::time::sleep(Duration::from_millis(50)).await; assert!(engine.runtime.is_dirty("pc-1"), "pc-1 never marked"); - assert!(engine.runtime.is_dirty("pc-2"), "a lagged window must mark every live terminal"); + assert!( + engine.runtime.is_dirty("pc-2"), + "a lagged window must mark every live terminal" + ); engine.stop(); tokio::time::sleep(Duration::from_millis(BASE_TICK_MS * 3)).await; - assert!(tap.is_finished(), "the tap must return once `stopping` is set"); + assert!( + tap.is_finished(), + "the tap must return once `stopping` is set" + ); drop(tx); } @@ -1197,15 +1820,22 @@ mod tests { #[test] fn the_tap_body_never_reads_the_payload_bytes() { let source = strip_comments(include_str!("loops.rs")); - let start = source.find("pub async fn run_tap(").expect("run_tap must exist"); + let start = source + .find("pub async fn run_tap(") + .expect("run_tap must exist"); let rest = &source[start..]; - let end = rest.find("\n}\n").expect("its body must be closed at column zero"); + let end = rest + .find("\n}\n") + .expect("its body must be closed at column zero"); let body = &rest[..end]; assert!( !body.contains(".data"), "the tap carries a SIGNAL, not data: the parser already has every byte, losslessly" ); - assert!(body.contains("mark_dirty"), "and it must actually mark something"); + assert!( + body.contains("mark_dirty"), + "and it must actually mark something" + ); } // ============================================================================================= @@ -1227,13 +1857,19 @@ mod tests { let targeting = tokio::spawn(run_targeting(engine.clone(), host.clone())); // The tap does work. - let _ = tx.send(ChannelPayload { id: "pc-1".into(), data: vec![b'x'] }); + let _ = tx.send(ChannelPayload { + id: "pc-1".into(), + data: vec![b'x'], + }); tokio::time::sleep(Duration::from_millis(50)).await; assert!(engine.runtime.is_dirty("pc-1"), "the tap did nothing"); // The targeting tick does work: it re-resolves `All terminals` and adopts tm-1. tokio::time::sleep(Duration::from_millis(TARGETING_TICK_MS)).await; - assert!(engine.runtime.watches("au-1", "tm-1"), "the targeting tick did nothing"); + assert!( + engine.runtime.watches("au-1", "tm-1"), + "the targeting tick did nothing" + ); // And the evaluator does work: with a terminal watched and dirty, the pair evaluates. tokio::time::sleep(Duration::from_millis(BASE_TICK_MS * 4)).await; @@ -1265,12 +1901,21 @@ mod tests { let cursor = evaluate_tick(&engine, &host, 0, 1_000).await; assert_eq!(cursor, 0); - assert!(fake.written().is_empty(), "a first sight must arm, never type"); + assert!( + fake.written().is_empty(), + "a first sight must arm, never type" + ); assert_eq!(engine.runtime.arm_state("au-1", "tm-1"), ArmState::armed()); // Nothing in the log, and that is the verbose gate doing its job: an ordinary check is the // outcome of most evaluations and would otherwise write four rows a second per pair. - assert!(log_kinds(&fake.store).is_empty(), "an ungated check would flood the log"); - assert!(!engine.runtime.is_dirty("pc-1"), "the only pair on pc-1 ran, so its flag is spent"); + assert!( + log_kinds(&fake.store).is_empty(), + "an ungated check would flood the log" + ); + assert!( + !engine.runtime.is_dirty("pc-1"), + "the only pair on pc-1 ran, so its flag is spent" + ); // The crossing. `dirty` again, and past the 250 ms floor. engine.runtime.mark_dirty("pc-1"); @@ -1281,7 +1926,9 @@ mod tests { let writes = fake.written(); assert!( - writes.iter().any(|w| w.contains("prepare to do context-hand-off")), + writes + .iter() + .any(|w| w.contains("prepare to do context-hand-off")), "the message was never typed: {:?}", writes ); @@ -1300,10 +1947,14 @@ mod tests { // message had even been typed — so the next tick read the rule's own echo as organic output. let gap = crate::automation::send::PASTE_SUBMIT_GAP_MS as i64; assert!( - engine.runtime.is_settling("tm-1", 2_000 + ECHO_SETTLE_MS + 1), + engine + .runtime + .is_settling("tm-1", 2_000 + ECHO_SETTLE_MS + 1), "the window closed a paste-to-submit gap too early" ); - assert!(!engine.runtime.is_settling("tm-1", 2_000 + gap + ECHO_SETTLE_MS + 1)); + assert!(!engine + .runtime + .is_settling("tm-1", 2_000 + gap + ECHO_SETTLE_MS + 1)); } // ============================================================================================= @@ -1319,8 +1970,8 @@ mod tests { let (engine, fake, host) = rig_with_rule(|g| { g.parse_mut().find = r"FAILED (\d+) tests in (\S+)".into(); g.cond_mut().finds = Finds::Event; - g.action.message = "Fix the $1 failing tests in $2".into(); - g.action.substitute = true; + g.action_mut().message = "Fix the $1 failing tests in $2".into(); + g.action_mut().substitute = true; }); engine.runtime.set_arm("au-1", "tm-1", ArmState::armed()); engine.runtime.mark_dirty("pc-1"); @@ -1330,7 +1981,9 @@ mod tests { tokio::time::sleep(Duration::from_millis(1_500)).await; assert!( - fake.written().iter().any(|w| w.contains("Fix the 17 failing tests in a.ts")), + fake.written() + .iter() + .any(|w| w.contains("Fix the 17 failing tests in a.ts")), "the resolved message was never typed: {:?}", fake.written() ); @@ -1357,10 +2010,17 @@ mod tests { #[tokio::test(start_paused = true)] async fn a_schedule_rule_reads_nothing_sends_nothing_and_logs_nothing() { let (engine, fake, host) = wire(vec![schedule_only_rule("au-sched")]); - assert_eq!(engine.snapshot_live().len(), 1, "premise: the rule IS live and IS walked"); + assert_eq!( + engine.snapshot_live().len(), + 1, + "premise: the rule IS live and IS walked" + ); - fake.say("pc-1", "ctx:99% FAILED 3 tests -"); + fake.say( + "pc-1", + "ctx:99% FAILED 3 tests +", + ); // **What `wire` already wrote, before the ticks run.** `wire` reloads at epoch 0 and §7's // seeding writes one `held` row for a schedule whose minute is already past *in the // runner's own zone* — 19:00 the previous evening west of UTC, midnight on it. So an @@ -1373,7 +2033,11 @@ mod tests { } tokio::time::sleep(Duration::from_millis(1_500)).await; - assert!(fake.written().is_empty(), "a rule with no pattern typed something: {:?}", fake.written()); + assert!( + fake.written().is_empty(), + "a rule with no pattern typed something: {:?}", + fake.written() + ); assert_eq!( log_rows(&fake.store), before, @@ -1398,17 +2062,20 @@ mod tests { /// is identical; only the pattern is present. If the rig itself were broken, this would be /// silent too, and "sends nothing" would prove nothing at all. #[tokio::test(start_paused = true)] - async fn the_same_rig_with_a_pattern_does_send() { + async fn the_same_rig_with_a_pattern_does_send() { let (engine, fake, host) = rig_with_rule(|g| { g.parse_mut().find = "FAILED".into(); g.parse_mut().keep = Keep::Whole; g.cond_mut().finds = Finds::Event; - g.action.message = "stand-up notes?".into(); + g.action_mut().message = "stand-up notes?".into(); }); engine.runtime.set_arm("au-1", "tm-1", ArmState::armed()); - fake.say("pc-1", "ctx:99% FAILED 3 tests -"); + fake.say( + "pc-1", + "ctx:99% FAILED 3 tests +", + ); for tick in 0..8 { engine.runtime.mark_dirty("pc-1"); evaluate_tick(&engine, &host, 0, 1_000 + tick * 250).await; @@ -1450,7 +2117,9 @@ mod tests { /// The local ordinal `at_local`'s day maps to — `last_fired_day`'s key. fn day_ordinal(y: i32, m: u32, d: u32) -> i32 { - NaiveDate::from_ymd_opt(y, m, d).expect("a real date").num_days_from_ce() + NaiveDate::from_ymd_opt(y, m, d) + .expect("a real date") + .num_days_from_ce() } /// A rig with several terminals and each rule's watched set given explicitly. @@ -1500,7 +2169,9 @@ mod tests { } for rule in &rules { if bypass_enable_gate { - fake.store.save_rule_bypassing_the_enable_gate_for_tests(rule).unwrap(); + fake.store + .save_rule_bypassing_the_enable_gate_for_tests(rule) + .unwrap(); } else { fake.store.save_rule(rule).unwrap(); } @@ -1508,7 +2179,9 @@ mod tests { let engine = Arc::new(AutomationEngine::new(0)); engine.reload(&fake.store, 0).unwrap(); for (id, leaves) in watched { - engine.runtime.set_watched(id, leaves.iter().map(|tm| tm.to_string()).collect()); + engine + .runtime + .set_watched(id, leaves.iter().map(|tm| tm.to_string()).collect()); } let host: Arc = fake.clone(); (engine, fake, host) @@ -1530,9 +2203,20 @@ mod tests { #[tokio::test(start_paused = true)] async fn a_schedule_rule_sends_to_every_target_when_the_minute_arrives_and_reads_nothing() { let (engine, fake, host) = wire_targets( - vec![schedule_only_rule("au-sched"), ctx_rule_saying("au-read", "a reader", 2)], - &[("tm-1", "pc-1"), ("tm-2", "pc-2"), ("tm-3", "pc-3"), ("tm-4", "pc-4")], - &[("au-sched", &["tm-1", "tm-2", "tm-3"]), ("au-read", &["tm-4"])], + vec![ + schedule_only_rule("au-sched"), + ctx_rule_saying("au-read", "a reader", 2), + ], + &[ + ("tm-1", "pc-1"), + ("tm-2", "pc-2"), + ("tm-3", "pc-3"), + ("tm-4", "pc-4"), + ], + &[ + ("au-sched", &["tm-1", "tm-2", "tm-3"]), + ("au-read", &["tm-4"]), + ], ); // The reader has output and sits below its threshold, so it reads, arms, and sends nothing. fake.say("pc-4", "ctx:5%\n"); @@ -1562,8 +2246,16 @@ mod tests { // 6.3: a schedule rule has no arm state and must not disturb one. for tm in ["tm-1", "tm-2", "tm-3"] { - assert_eq!(engine.runtime.arm_state("au-sched", tm), ArmState::Unseen, "{tm} armed"); - assert_eq!(engine.runtime.last_eval("au-sched", tm), None, "{tm} was evaluated"); + assert_eq!( + engine.runtime.arm_state("au-sched", tm), + ArmState::Unseen, + "{tm} armed" + ); + assert_eq!( + engine.runtime.last_eval("au-sched", tm), + None, + "{tm} was evaluated" + ); } assert_eq!( engine.runtime.last_fired_day("au-sched"), @@ -1619,7 +2311,10 @@ mod tests { &[("tm-1", "pc-1"), ("tm-3", "pc-3")], &[("au-sched", &["tm-1", "tm-2", "tm-3"])], ); - assert!(host.process_for_leaf("tm-2").is_none(), "premise: tm-2 is dormant"); + assert!( + host.process_for_leaf("tm-2").is_none(), + "premise: tm-2 is dormant" + ); evaluate_tick(&engine, &host, 0, at_local(2026, 9, 7, Weekday::Mon, 9, 0)).await; tokio::time::sleep(Duration::from_millis(2_000)).await; @@ -1651,9 +2346,15 @@ mod tests { /// was killed outright. #[tokio::test(start_paused = true)] async fn a_schedule_whose_targets_were_all_asleep_does_not_nag_the_first_one_to_wake() { - let (engine, fake, host) = - wire_targets(vec![schedule_only_rule("au-sched")], &[], &[("au-sched", &["tm-1"])]); - assert!(host.process_for_leaf("tm-1").is_none(), "premise: nothing is awake"); + let (engine, fake, host) = wire_targets( + vec![schedule_only_rule("au-sched")], + &[], + &[("au-sched", &["tm-1"])], + ); + assert!( + host.process_for_leaf("tm-1").is_none(), + "premise: nothing is awake" + ); evaluate_tick(&engine, &host, 0, at_local(2026, 9, 7, Weekday::Mon, 9, 0)).await; tokio::time::sleep(Duration::from_millis(2_000)).await; @@ -1726,11 +2427,22 @@ mod tests { let tuesday_afternoon = at_local(2026, 9, 8, Weekday::Tue, 14, 0); evaluator_step(&engine, &host, 0, Some(woke_at), tuesday_afternoon).await; tokio::time::sleep(Duration::from_millis(2_000)).await; - assert!(fake.written().is_empty(), "delivered later the same day: {:?}", fake.written()); + assert!( + fake.written().is_empty(), + "delivered later the same day: {:?}", + fake.written() + ); // Wednesday, with the app genuinely awake across the minute. let wednesday = at_local(2026, 9, 9, Weekday::Wed, 9, 0); - evaluator_step(&engine, &host, 0, Some(wednesday - BASE_TICK_MS as i64), wednesday).await; + evaluator_step( + &engine, + &host, + 0, + Some(wednesday - BASE_TICK_MS as i64), + wednesday, + ) + .await; tokio::time::sleep(Duration::from_millis(2_000)).await; assert_eq!( sent_to(&fake, "stand-up notes?"), @@ -1772,14 +2484,21 @@ mod tests { tokio::time::sleep(Duration::from_millis(2_000)).await; let rows: Vec<_> = log_rows(&fake.store).split_off(before); - assert_eq!(rows.len(), 1, "the wake spent Tuesday and said nothing: {rows:?}"); + assert_eq!( + rows.len(), + 1, + "the wake spent Tuesday and said nothing: {rows:?}" + ); assert_eq!(rows[0].0, "Held", "{rows:?}"); assert_eq!( rows[0].1, "09:00 went by while nothing was watching the clock, so today's run was skipped", "{rows:?}" ); - assert_eq!(rows[0].2, None, "a schedule's suppression names no terminal: {rows:?}"); + assert_eq!( + rows[0].2, None, + "a schedule's suppression names no terminal: {rows:?}" + ); assert!( fake.activity.load(std::sync::atomic::Ordering::Relaxed) > emits_before, "the row was written and no window was told to refetch the log" @@ -1817,7 +2536,12 @@ mod tests { evaluator_step(&engine, &host, 0, Some(nine - BASE_TICK_MS as i64), nine).await; tokio::time::sleep(Duration::from_millis(2_000)).await; - assert_eq!(sent_to(&fake, "stand-up notes?"), vec!["pc-1"], "{:?}", fake.written()); + assert_eq!( + sent_to(&fake, "stand-up notes?"), + vec!["pc-1"], + "{:?}", + fake.written() + ); let rows: Vec<_> = log_rows(&fake.store).split_off(before); assert!( rows.iter().all(|(kind, _, _)| kind != "Held"), @@ -1953,14 +2677,19 @@ mod tests { async fn a_failed_schedule_send_rolls_back_to_what_was_there_and_still_names_the_terminal() { let mut hybrid = ctx_rule_saying("au-both", "stand-up notes?", 1); hybrid.graph.timer = Some(TimerStep { - mode: TimerMode::DailyAt { minute_of_day: 9 * 60, days: 0b0001_1111 }, + mode: TimerMode::DailyAt { + minute_of_day: 9 * 60, + days: 0b0001_1111, + }, }); let (engine, fake, host) = wire_targets_bypassing_the_enable_gate( vec![hybrid], &[("tm-1", "pc-1")], &[("au-both", &["tm-1"])], ); - engine.runtime.set_arm("au-both", "tm-1", ArmState::Fired { at_ms: 5 }); + engine + .runtime + .set_arm("au-both", "tm-1", ArmState::Fired { at_ms: 5 }); // `wire`'s own reload may have written §7's suppression row already, depending on the // runner's zone — see `a_schedule_rule_reads_nothing_sends_nothing_and_logs_nothing`. The // rows this test is about are the ones the tick adds. @@ -1971,7 +2700,11 @@ mod tests { fake.close("tm-1"); tokio::time::sleep(Duration::from_millis(2_000)).await; - assert!(fake.written().is_empty(), "{:?} reached a closed terminal", fake.written()); + assert!( + fake.written().is_empty(), + "{:?} reached a closed terminal", + fake.written() + ); assert_eq!( engine.runtime.arm_state("au-both", "tm-1"), ArmState::Fired { at_ms: 5 }, @@ -1981,7 +2714,9 @@ mod tests { assert_eq!(rows.len(), 1, "exactly one row: {rows:?}"); assert_eq!(rows[0].0, "Failed", "{rows:?}"); assert!( - rows[0].1.contains("the terminal closed before the message was sent"), + rows[0] + .1 + .contains("the terminal closed before the message was sent"), "{rows:?}" ); assert_eq!( @@ -2039,7 +2774,10 @@ mod tests { async fn a_schedule_rule_that_also_has_a_monitor_reads_nothing_and_fires_on_the_clock() { let mut hybrid = ctx_rule_saying("au-both", "stand-up notes?", 1); hybrid.graph.timer = Some(TimerStep { - mode: TimerMode::DailyAt { minute_of_day: 9 * 60, days: 0b0001_1111 }, + mode: TimerMode::DailyAt { + minute_of_day: 9 * 60, + days: 0b0001_1111, + }, }); let (engine, fake, host) = wire_targets_bypassing_the_enable_gate( vec![hybrid, ctx_rule_saying("au-read", "a reader", 2)], @@ -2060,7 +2798,11 @@ mod tests { "the monitor crossed on a rule the clock has not reached: {:?}", fake.written() ); - assert_eq!(fake.tailed(), vec!["pc-4"], "and the control read a window to prove it could"); + assert_eq!( + fake.tailed(), + vec!["pc-4"], + "and the control read a window to prove it could" + ); engine.runtime.mark_dirty("pc-1"); engine.runtime.mark_dirty("pc-4"); @@ -2092,8 +2834,8 @@ mod tests { let (engine, fake, host) = rig_with_rule(|g| { g.parse_mut().find = r"FAILED (\d+)".into(); g.cond_mut().finds = Finds::Event; - g.action.message = "awk '{print $1}'".into(); - g.action.substitute = false; + g.action_mut().message = "awk '{print $1}'".into(); + g.action_mut().substitute = false; }); engine.runtime.set_arm("au-1", "tm-1", ArmState::armed()); engine.runtime.mark_dirty("pc-1"); @@ -2103,7 +2845,9 @@ mod tests { tokio::time::sleep(Duration::from_millis(1_500)).await; assert!( - fake.written().iter().any(|w| w.contains("awk '{print $1}'")), + fake.written() + .iter() + .any(|w| w.contains("awk '{print $1}'")), "the literal message was never typed: {:?}", fake.written() ); @@ -2123,8 +2867,8 @@ mod tests { let (engine, fake, host) = rig_with_rule_bypassing_the_enable_gate(|g| { g.parse_mut().find = r"FAILED (\d+)".into(); g.cond_mut().finds = Finds::Event; - g.action.message = "Fix $3".into(); - g.action.substitute = true; + g.action_mut().message = "Fix $3".into(); + g.action_mut().substitute = true; }); engine.runtime.set_arm("au-1", "tm-1", ArmState::armed()); engine.runtime.mark_dirty("pc-1"); @@ -2133,13 +2877,20 @@ mod tests { evaluate_tick(&engine, &host, 0, 1_000).await; tokio::time::sleep(Duration::from_millis(1_500)).await; - assert!(fake.written().is_empty(), "nothing may be typed: {:?}", fake.written()); + assert!( + fake.written().is_empty(), + "nothing may be typed: {:?}", + fake.written() + ); let log = log_details(&fake.store); assert!( log.iter().any(|(_, detail)| detail.contains("$3")), "the failure row must name the token, got: {log:?}" ); - assert!(log.iter().any(|(kind, _)| kind == "Failed"), "and it must be a Failed row: {log:?}"); + assert!( + log.iter().any(|(kind, _)| kind == "Failed"), + "and it must be a Failed row: {log:?}" + ); } // ============================================================================================= @@ -2162,8 +2913,10 @@ mod tests { let (engine, fake, host) = rig_with_rule(|g| { g.parse_mut().find = "API error".into(); g.cond_mut().finds = Finds::Event; - g.action.message = "resume".into(); - g.timer = Some(TimerStep { mode: TimerMode::AfterMatch { delay_ms: 30_000 } }); + g.action_mut().message = "resume".into(); + g.timer = Some(TimerStep { + mode: TimerMode::AfterMatch { delay_ms: 30_000 }, + }); }); engine.runtime.set_arm("au-1", "tm-1", ArmState::armed()); engine.runtime.mark_dirty("pc-1"); @@ -2179,7 +2932,11 @@ mod tests { evaluate_tick(&engine, &host, 0, 20_000).await; tokio::time::sleep(Duration::from_millis(1_500)).await; - assert!(fake.written().is_empty(), "still holding at 19s: {:?}", fake.written()); + assert!( + fake.written().is_empty(), + "still holding at 19s: {:?}", + fake.written() + ); evaluate_tick(&engine, &host, 0, 31_001).await; tokio::time::sleep(Duration::from_millis(2_000)).await; @@ -2204,8 +2961,10 @@ mod tests { let (engine, fake, host) = rig_with_rule(|g| { g.parse_mut().find = "API error".into(); g.cond_mut().finds = Finds::Event; - g.action.message = "resume".into(); - g.timer = Some(TimerStep { mode: TimerMode::AfterMatch { delay_ms: 30_000 } }); + g.action_mut().message = "resume".into(); + g.timer = Some(TimerStep { + mode: TimerMode::AfterMatch { delay_ms: 30_000 }, + }); }); engine.runtime.set_arm("au-1", "tm-1", ArmState::armed()); engine.runtime.mark_dirty("pc-1"); @@ -2244,7 +3003,12 @@ mod tests { // `due_at_ms` is the moment it may go, not the moment after. evaluate_tick(&engine, &host, 0, 31_000).await; tokio::time::sleep(Duration::from_millis(2_000)).await; - assert_eq!(times_sent(&fake, "resume"), 1, "exactly one message: {:?}", fake.written()); + assert_eq!( + times_sent(&fake, "resume"), + 1, + "exactly one message: {:?}", + fake.written() + ); assert_eq!( engine.runtime.parked_at("au-1", "tm-1"), None, @@ -2264,8 +3028,10 @@ mod tests { let (engine, fake, host) = rig_with_rule(|g| { g.parse_mut().find = "API error".into(); g.cond_mut().finds = Finds::Event; - g.action.message = "resume".into(); - g.timer = Some(TimerStep { mode: TimerMode::AfterMatch { delay_ms: 30_000 } }); + g.action_mut().message = "resume".into(); + g.timer = Some(TimerStep { + mode: TimerMode::AfterMatch { delay_ms: 30_000 }, + }); }); engine.runtime.set_arm("au-1", "tm-1", ArmState::armed()); engine.runtime.mark_dirty("pc-1"); @@ -2313,8 +3079,10 @@ mod tests { let (engine, fake, host) = rig_with_rule(|g| { g.parse_mut().find = "API error".into(); g.cond_mut().finds = Finds::Event; - g.action.message = "resume".into(); - g.timer = Some(TimerStep { mode: TimerMode::AfterMatch { delay_ms: 300_000 } }); + g.action_mut().message = "resume".into(); + g.timer = Some(TimerStep { + mode: TimerMode::AfterMatch { delay_ms: 300_000 }, + }); }); engine.runtime.set_arm("au-1", "tm-1", ArmState::armed()); engine.runtime.mark_dirty("pc-1"); @@ -2323,7 +3091,11 @@ mod tests { // The crossing parks a send due at 301_000 (five minutes out). evaluator_step(&engine, &host, 0, None, 1_000).await; tokio::time::sleep(Duration::from_millis(500)).await; - assert_eq!(engine.runtime.parked_at("au-1", "tm-1"), Some(301_000), "premise: it is parked"); + assert_eq!( + engine.runtime.parked_at("au-1", "tm-1"), + Some(301_000), + "premise: it is parked" + ); // A resume that drops nothing: over `RESUME_GAP_MS`, and the send is not yet even due. let quiet = fake.states.load(std::sync::atomic::Ordering::Relaxed); @@ -2364,8 +3136,10 @@ mod tests { let (engine, fake, host) = rig_with_rule(|g| { g.parse_mut().find = "API error".into(); g.cond_mut().finds = Finds::Event; - g.action.message = "resume".into(); - g.timer = Some(TimerStep { mode: TimerMode::AfterMatch { delay_ms: 300_000 } }); + g.action_mut().message = "resume".into(); + g.timer = Some(TimerStep { + mode: TimerMode::AfterMatch { delay_ms: 300_000 }, + }); }); engine.runtime.set_arm("au-1", "tm-1", ArmState::armed()); engine.runtime.mark_dirty("pc-1"); @@ -2385,7 +3159,11 @@ mod tests { Some(301_000), "a resume dropped a send that was not yet stale" ); - assert!(fake.written().is_empty(), "the send is not due yet: {:?}", fake.written()); + assert!( + fake.written().is_empty(), + "the send is not due yet: {:?}", + fake.written() + ); // An ordinary tick, once the wait is genuinely over, must still deliver it. evaluator_step(&engine, &host, 0, Some(120_000), 301_001).await; @@ -2409,8 +3187,10 @@ mod tests { let (engine, fake, host) = rig_with_rule(|g| { g.parse_mut().find = "API error".into(); g.cond_mut().finds = Finds::Event; - g.action.message = "resume".into(); - g.timer = Some(TimerStep { mode: TimerMode::AfterMatch { delay_ms: 30_000 } }); + g.action_mut().message = "resume".into(); + g.timer = Some(TimerStep { + mode: TimerMode::AfterMatch { delay_ms: 30_000 }, + }); }); engine.runtime.set_arm("au-1", "tm-1", ArmState::armed()); engine.runtime.mark_dirty("pc-1"); @@ -2459,15 +3239,20 @@ mod tests { let (engine, fake, host) = rig_with_rule(|g| { g.parse_mut().find = r"API error (\d+)".into(); g.cond_mut().finds = Finds::Event; - g.action.message = "resume after $1".into(); - g.action.substitute = true; - g.timer = Some(TimerStep { mode: TimerMode::AfterMatch { delay_ms: 30_000 } }); + g.action_mut().message = "resume after $1".into(); + g.action_mut().substitute = true; + g.timer = Some(TimerStep { + mode: TimerMode::AfterMatch { delay_ms: 30_000 }, + }); }); engine.runtime.set_arm("au-1", "tm-1", ArmState::armed()); engine.runtime.mark_dirty("pc-1"); fake.say("pc-1", "API error 529"); evaluate_tick(&engine, &host, 0, 1_000).await; - assert!(fake.written().is_empty(), "the premise: the crossing parked rather than sending"); + assert!( + fake.written().is_empty(), + "the premise: the crossing parked rather than sending" + ); // Half a minute of build output later, nothing of the match is left anywhere. fake.say("pc-1", "all clear\n"); @@ -2505,8 +3290,10 @@ mod tests { let (engine, fake, host) = rig_with_rule(|g| { g.parse_mut().find = "API error".into(); g.cond_mut().finds = Finds::Event; - g.action.message = "resume".into(); - g.timer = Some(TimerStep { mode: TimerMode::AfterMatch { delay_ms: 30_000 } }); + g.action_mut().message = "resume".into(); + g.timer = Some(TimerStep { + mode: TimerMode::AfterMatch { delay_ms: 30_000 }, + }); }); engine.runtime.set_arm("au-1", "tm-1", ArmState::armed()); engine.runtime.mark_dirty("pc-1"); @@ -2522,7 +3309,10 @@ mod tests { if restarted { // A spawn re-indexing a LIVE leaf, which is all `IdentityIndex::index` does. Not a // `forget_terminal`, because that is the path this hazard is NOT on. - fake.leaves.lock().unwrap().insert("tm-1".into(), "pc-2".into()); + fake.leaves + .lock() + .unwrap() + .insert("tm-1".into(), "pc-2".into()); } evaluate_tick(&engine, &host, 0, 31_001).await; @@ -2567,8 +3357,10 @@ mod tests { let (engine, fake, host) = rig_with_rule(|g| { g.parse_mut().find = "API error".into(); g.cond_mut().finds = Finds::Event; - g.action.message = "resume".into(); - g.timer = Some(TimerStep { mode: TimerMode::AfterMatch { delay_ms: 30_000 } }); + g.action_mut().message = "resume".into(); + g.timer = Some(TimerStep { + mode: TimerMode::AfterMatch { delay_ms: 30_000 }, + }); }); engine.runtime.set_arm("au-1", "tm-1", ArmState::armed()); engine.runtime.mark_dirty("pc-1"); @@ -2577,11 +3369,18 @@ mod tests { evaluate_tick(&engine, &host, 0, 1_000).await; // Guard against the vacuous version: a test that never parked anything would trivially type // nothing and stay green forever. - assert_eq!(engine.runtime.parked_at("au-1", "tm-1"), Some(31_000), "the premise: it parked"); + assert_eq!( + engine.runtime.parked_at("au-1", "tm-1"), + Some(31_000), + "the premise: it parked" + ); fake.store.set_enabled_checked("au-1", false).unwrap(); engine.reload(&fake.store, 2_000).unwrap(); - assert!(!engine.is_live("au-1"), "the premise: disabling drops it from the live set"); + assert!( + !engine.is_live("au-1"), + "the premise: disabling drops it from the live set" + ); evaluate_tick(&engine, &host, 0, 31_001).await; tokio::time::sleep(Duration::from_millis(2_000)).await; @@ -2599,19 +3398,31 @@ mod tests { let (engine, fake, host) = rig_with_rule(|g| { g.parse_mut().find = "API error".into(); g.cond_mut().finds = Finds::Event; - g.action.message = "resume".into(); - g.timer = Some(TimerStep { mode: TimerMode::AfterMatch { delay_ms: 30_000 } }); + g.action_mut().message = "resume".into(); + g.timer = Some(TimerStep { + mode: TimerMode::AfterMatch { delay_ms: 30_000 }, + }); }); engine.runtime.set_arm("au-1", "tm-1", ArmState::armed()); engine.runtime.mark_dirty("pc-1"); fake.say("pc-1", "API error"); evaluate_tick(&engine, &host, 0, 1_000).await; - assert_eq!(engine.runtime.parked_at("au-1", "tm-1"), Some(31_000), "the premise: it parked"); + assert_eq!( + engine.runtime.parked_at("au-1", "tm-1"), + Some(31_000), + "the premise: it parked" + ); - assert!(fake.store.delete_rule("au-1").unwrap(), "the premise: the rule existed to delete"); + assert!( + fake.store.delete_rule("au-1").unwrap(), + "the premise: the rule existed to delete" + ); engine.reload(&fake.store, 2_000).unwrap(); - assert!(!engine.is_live("au-1"), "the premise: a deleted rule is not live"); + assert!( + !engine.is_live("au-1"), + "the premise: a deleted rule is not live" + ); evaluate_tick(&engine, &host, 0, 31_001).await; tokio::time::sleep(Duration::from_millis(2_000)).await; @@ -2630,18 +3441,27 @@ mod tests { let (engine, fake, host) = rig_with_rule(|g| { g.parse_mut().find = "API error".into(); g.cond_mut().finds = Finds::Event; - g.action.message = "resume".into(); - g.timer = Some(TimerStep { mode: TimerMode::AfterMatch { delay_ms: 30_000 } }); + g.action_mut().message = "resume".into(); + g.timer = Some(TimerStep { + mode: TimerMode::AfterMatch { delay_ms: 30_000 }, + }); }); engine.runtime.set_arm("au-1", "tm-1", ArmState::armed()); engine.runtime.mark_dirty("pc-1"); fake.say("pc-1", "API error"); evaluate_tick(&engine, &host, 0, 1_000).await; - assert_eq!(engine.runtime.parked_at("au-1", "tm-1"), Some(31_000), "the premise: it parked"); + assert_eq!( + engine.runtime.parked_at("au-1", "tm-1"), + Some(31_000), + "the premise: it parked" + ); - let mut edited = - fake.store.get_rule("au-1").unwrap().expect("the rule must still be in the store"); + let mut edited = fake + .store + .get_rule("au-1") + .unwrap() + .expect("the rule must still be in the store"); edited.updated_at = 2_000; fake.store.save_rule(&edited).unwrap(); engine.reload(&fake.store, 2_000).unwrap(); @@ -2654,7 +3474,9 @@ mod tests { // `TARGETING_TICK_MS` — that tick does not run in this harness. Re-establishing it here is // NOT the thing under test; skipping it would let the walk skip "tm-1" for a reason that has // nothing to do with §6.1, and the test would pass vacuously for the wrong reason. - engine.runtime.set_watched("au-1", ["tm-1".to_string()].into()); + engine + .runtime + .set_watched("au-1", ["tm-1".to_string()].into()); evaluate_tick(&engine, &host, 0, 31_001).await; tokio::time::sleep(Duration::from_millis(2_000)).await; @@ -2676,10 +3498,14 @@ mod tests { once.runs_once = true; once.graph.parse_mut().find = "API error".into(); once.graph.cond_mut().finds = Finds::Event; - once.graph.timer = Some(TimerStep { mode: TimerMode::AfterMatch { delay_ms: 30_000 } }); + once.graph.timer = Some(TimerStep { + mode: TimerMode::AfterMatch { delay_ms: 30_000 }, + }); let (engine, fake, host) = wire(vec![once]); open_second_terminal(&fake); - engine.runtime.set_watched("au-once", ["tm-1".to_string(), "tm-2".to_string()].into()); + engine + .runtime + .set_watched("au-once", ["tm-1".to_string(), "tm-2".to_string()].into()); for tm in ["tm-1", "tm-2"] { engine.runtime.set_arm("au-once", tm, ArmState::armed()); } @@ -2691,7 +3517,11 @@ mod tests { engine.runtime.mark_dirty("pc-2"); evaluate_tick(&engine, &host, 0, 1_250).await; assert_eq!(engine.runtime.parked_at("au-once", "tm-1"), Some(31_000)); - assert_eq!(engine.runtime.parked_at("au-once", "tm-2"), Some(31_250), "both parked"); + assert_eq!( + engine.runtime.parked_at("au-once", "tm-2"), + Some(31_250), + "both parked" + ); evaluate_tick(&engine, &host, 0, 31_250).await; tokio::time::sleep(Duration::from_millis(4_000)).await; @@ -2727,10 +3557,14 @@ mod tests { once.runs_once = true; once.graph.parse_mut().find = "API error".into(); once.graph.cond_mut().finds = Finds::Event; - once.graph.timer = Some(TimerStep { mode: TimerMode::AfterMatch { delay_ms: 30_000 } }); + once.graph.timer = Some(TimerStep { + mode: TimerMode::AfterMatch { delay_ms: 30_000 }, + }); let (engine, fake, host) = wire(vec![once]); open_second_terminal(&fake); - engine.runtime.set_watched("au-once", ["tm-1".to_string(), "tm-2".to_string()].into()); + engine + .runtime + .set_watched("au-once", ["tm-1".to_string(), "tm-2".to_string()].into()); for tm in ["tm-1", "tm-2"] { engine.runtime.set_arm("au-once", tm, ArmState::armed()); } @@ -2742,14 +3576,27 @@ mod tests { engine.runtime.mark_dirty("pc-2"); evaluate_tick(&engine, &host, 0, 1_250).await; assert_eq!(engine.runtime.parked_at("au-once", "tm-1"), Some(31_000)); - assert_eq!(engine.runtime.parked_at("au-once", "tm-2"), Some(31_250), "premise: both parked"); + assert_eq!( + engine.runtime.parked_at("au-once", "tm-2"), + Some(31_250), + "premise: both parked" + ); evaluate_tick(&engine, &host, 0, 31_250).await; tokio::time::sleep(Duration::from_millis(4_000)).await; let sent = sent_to(&fake, "once only"); - assert_eq!(sent.len(), 1, "premise: R6 still lets exactly one through: {:?}", fake.written()); - let loser = if sent[0] == "pc-1" { "second" } else { "codex · core" }; + assert_eq!( + sent.len(), + 1, + "premise: R6 still lets exactly one through: {:?}", + fake.written() + ); + let loser = if sent[0] == "pc-1" { + "second" + } else { + "codex · core" + }; let dropped: Vec<_> = log_rows(&fake.store) .into_iter() @@ -2782,14 +3629,19 @@ mod tests { #[tokio::test(start_paused = true)] async fn a_dormant_terminal_produces_no_evaluation_and_no_log_line() { let (engine, fake, host) = wired(); - engine.runtime.set_arm("au-1", "tm-1", ArmState::Fired { at_ms: 5 }); + engine + .runtime + .set_arm("au-1", "tm-1", ArmState::Fired { at_ms: 5 }); engine.runtime.mark_dirty("pc-1"); // Watched, but the leaf resolves to nothing: session restore has not re-registered it. fake.close("tm-1"); evaluate_tick(&engine, &host, 0, 1_000).await; - assert_eq!(engine.runtime.arm_state("au-1", "tm-1"), ArmState::Fired { at_ms: 5 }); + assert_eq!( + engine.runtime.arm_state("au-1", "tm-1"), + ArmState::Fired { at_ms: 5 } + ); assert_eq!(engine.runtime.last_eval("au-1", "tm-1"), None); assert!(log_kinds(&fake.store).is_empty()); } @@ -2804,7 +3656,11 @@ mod tests { engine.runtime.settle_until("tm-1", 5_000); evaluate_tick(&engine, &host, 0, 1_000).await; - assert_eq!(engine.runtime.last_eval("au-1", "tm-1"), None, "settling means untouched"); + assert_eq!( + engine.runtime.last_eval("au-1", "tm-1"), + None, + "settling means untouched" + ); // And once the window closes it evaluates normally again. evaluate_tick(&engine, &host, 0, 5_001).await; @@ -2830,9 +3686,15 @@ mod tests { .into_iter() .find(|l| l.rule.id == rule_id) .expect("the rule must be live for a send to have been decided"); - engine.runtime.set_arm(rule_id, "tm-1", ArmState::Fired { at_ms }); + engine + .runtime + .set_arm(rule_id, "tm-1", ArmState::Fired { at_ms }); PendingSend { - pair: Pair { rule, tm: "tm-1".into(), pc: "pc-1".into() }, + pair: Pair { + rule, + tm: "tm-1".into(), + pc: "pc-1".into(), + }, prev, label: host.label_for("tm-1"), at_ms, @@ -2881,7 +3743,12 @@ mod tests { tokio::time::sleep(Duration::from_millis(2_000)).await; let writes = fake.written(); - assert_eq!(writes.len(), 6, "two sends of three writes each: {:?}", writes); + assert_eq!( + writes.len(), + 6, + "two sends of three writes each: {:?}", + writes + ); assert_eq!( paste_positions(&writes), vec![0, 3], @@ -2894,11 +3761,20 @@ mod tests { for i in [0usize, 3] { let alpha = writes[i].contains("alpha speaking"); let bravo = writes[i].contains("bravo speaking"); - assert_ne!(alpha, bravo, "one paste carried both messages: {:?}", writes); + assert_ne!( + alpha, bravo, + "one paste carried both messages: {:?}", + writes + ); carried.push(if alpha { "alpha" } else { "bravo" }); } carried.sort_unstable(); - assert_eq!(carried, vec!["alpha", "bravo"], "both rules must have sent: {:?}", writes); + assert_eq!( + carried, + vec!["alpha", "bravo"], + "both rules must have sent: {:?}", + writes + ); } /// A send that cannot take the terminal's queue within [`SEND_QUEUE_TIMEOUT_MS`] gives up: one @@ -2915,9 +3791,12 @@ mod tests { let _held = lock.lock().await; let send = pending(&engine, &host, "au-1", ArmState::re_armed(), 1_000); - run_send(engine.clone(), host.clone(), send).await; + run_crossing(engine.clone(), host.clone(), send).await; - assert!(fake.written().is_empty(), "a send that never got the queue must type nothing"); + assert!( + fake.written().is_empty(), + "a send that never got the queue must type nothing" + ); assert_eq!( engine.runtime.arm_state("au-1", "tm-1"), ArmState::re_armed(), @@ -2926,8 +3805,16 @@ mod tests { let rows = log_details(&fake.store); assert_eq!(rows.len(), 1, "exactly one row: {:?}", rows); assert_eq!(rows[0].0, "Failed"); - assert!(rows[0].1.contains("another rule was still sending"), "{:?}", rows); - assert_eq!(engine.runtime.fire_record("au-1", "tm-1"), None, "a failed send never fired"); + assert!( + rows[0].1.contains("another rule was still sending"), + "{:?}", + rows + ); + assert_eq!( + engine.runtime.fire_record("au-1", "tm-1"), + None, + "a failed send never fired" + ); } // ============================================================================================= @@ -2966,7 +3853,7 @@ mod tests { let send = pending(&engine, &host, "au-1", ArmState::armed(), 1_000); break_it(&fake); - run_send(engine.clone(), host.clone(), send).await; + run_crossing(engine.clone(), host.clone(), send).await; tokio::time::sleep(Duration::from_millis(1_500)).await; assert_eq!( @@ -2976,7 +3863,12 @@ mod tests { what, fake.written() ); - assert_eq!(engine.runtime.arm_state("au-1", "tm-1"), ArmState::armed(), "{}", what); + assert_eq!( + engine.runtime.arm_state("au-1", "tm-1"), + ArmState::armed(), + "{}", + what + ); let rows = log_details(&fake.store); assert_eq!(rows.len(), 1, "{}: exactly one row, got {:?}", what, rows); assert_eq!(rows[0].0, "Failed", "{}: {:?}", what, rows); @@ -3016,8 +3908,15 @@ mod tests { bad.graph.parse_mut().find = r"ctx:(\d+%".into(); let (engine, fake, host) = wire(vec![bad]); - assert_eq!(log_kinds(&fake.store), vec!["Failed".to_string()], "one row, written at load"); - assert!(engine.snapshot_live().is_empty(), "and the rule is not running"); + assert_eq!( + log_kinds(&fake.store), + vec!["Failed".to_string()], + "one row, written at load" + ); + assert!( + engine.snapshot_live().is_empty(), + "and the rule is not running" + ); fake.say("pc-1", "ctx:63%\n"); let mut cursor = 0; @@ -3026,8 +3925,15 @@ mod tests { cursor = evaluate_tick(&engine, &host, cursor, t * 1_000).await; } - assert_eq!(log_kinds(&fake.store), vec!["Failed".to_string()], "a tick wrote a second row"); - assert!(fake.written().is_empty(), "and an uncompilable rule must never send"); + assert_eq!( + log_kinds(&fake.store), + vec!["Failed".to_string()], + "a tick wrote a second row" + ); + assert!( + fake.written().is_empty(), + "and an uncompilable rule must never send" + ); } // ============================================================================================= @@ -3042,9 +3948,12 @@ mod tests { let send = pending(&engine, &host, "au-1", ArmState::armed(), 1_000); engine.stop(); - run_send(engine.clone(), host.clone(), send).await; + run_crossing(engine.clone(), host.clone(), send).await; - assert!(fake.written().is_empty(), "a send that started after the quit must type nothing"); + assert!( + fake.written().is_empty(), + "a send that started after the quit must type nothing" + ); assert_eq!( engine.runtime.arm_state("au-1", "tm-1"), ArmState::armed(), @@ -3056,7 +3965,11 @@ mod tests { // the arm state is restored, the crossing is still armed, and it fires on the next launch. // A row here would be the only trace of a non-event, in a 200-row log §3.3 reserves for // decisions a user can act on. - assert!(log_kinds(&fake.store).is_empty(), "{:?}", log_details(&fake.store)); + assert!( + log_kinds(&fake.store).is_empty(), + "{:?}", + log_details(&fake.store) + ); } // Already typing: the flag is set during the paste-to-submit gap and the submit still goes @@ -3064,10 +3977,14 @@ mod tests { { let (engine, fake, host) = wired(); let send = pending(&engine, &host, "au-1", ArmState::armed(), 1_000); - let task = tokio::spawn(run_send(engine.clone(), host.clone(), send)); + let task = tokio::spawn(run_crossing(engine.clone(), host.clone(), send)); tokio::time::sleep(Duration::from_millis(100)).await; - assert_eq!(fake.written().len(), 1, "the paste must be out and the gap running"); + assert_eq!( + fake.written().len(), + 1, + "the paste must be out and the gap running" + ); engine.stop(); tokio::time::sleep(Duration::from_millis(1_000)).await; assert!(task.is_finished(), "the send must not park on the flag"); @@ -3104,8 +4021,16 @@ mod tests { evaluate_tick(&engine, &host, 0, 1_000).await; tokio::time::sleep(Duration::from_millis(1_500)).await; - assert_eq!(fake.written().len(), 3, "the crossing must send: {:?}", fake.written()); - assert_eq!(engine.runtime.echoes_for("tm-1", 1_000), vec!["HANDOFF now".to_string()]); + assert_eq!( + fake.written().len(), + 3, + "the crossing must send: {:?}", + fake.written() + ); + assert_eq!( + engine.runtime.echoes_for("tm-1", 1_000), + vec!["HANDOFF now".to_string()] + ); // The real line has scrolled off. All that is left is what this rule typed. fake.say("pc-1", "HANDOFF now\n"); @@ -3213,7 +4138,10 @@ mod tests { evaluate_tick(&engine, &host, 0, 1_000).await; tokio::time::sleep(Duration::from_millis(2_000)).await; - assert!(!engine.is_live("au-once"), "§7.8: completion is an in-memory event FIRST"); + assert!( + !engine.is_live("au-once"), + "§7.8: completion is an in-memory event FIRST" + ); assert_eq!( engine.runtime.arm_state("au-once", "tm-1"), ArmState::Unseen, @@ -3246,7 +4174,11 @@ mod tests { tokio::time::sleep(Duration::from_millis(2_000)).await; } - assert_eq!(times_sent(&fake, "once only"), 1, "a runs-once rule fired twice in one session"); + assert_eq!( + times_sent(&fake, "once only"), + 1, + "a runs-once rule fired twice in one session" + ); assert_eq!( times_sent(&fake, "every time"), 2, @@ -3256,16 +4188,27 @@ mod tests { // (b) The next launch: a fresh engine loads the same store and must not run it at all. let next = Arc::new(AutomationEngine::new(0)); next.reload(&fake.store, 6_000).unwrap(); - assert!(!next.is_live("au-once"), "the reload filter is the second line of defence"); - assert!(next.is_live("au-many"), "and only the completed rule is filtered"); + assert!( + !next.is_live("au-once"), + "the reload filter is the second line of defence" + ); + assert!( + next.is_live("au-many"), + "and only the completed rule is filtered" + ); - next.runtime.set_watched("au-once", ["tm-1".to_string()].into()); + next.runtime + .set_watched("au-once", ["tm-1".to_string()].into()); next.runtime.set_arm("au-once", "tm-1", ArmState::armed()); fake.say("pc-1", "ctx:77%\n"); next.runtime.mark_dirty("pc-1"); evaluate_tick(&next, &host, 0, 7_000).await; tokio::time::sleep(Duration::from_millis(2_000)).await; - assert_eq!(times_sent(&fake, "once only"), 1, "a completed rule ran after a reload"); + assert_eq!( + times_sent(&fake, "once only"), + 1, + "a completed rule ran after a reload" + ); } /// **The completion nobody was told about.** @@ -3293,14 +4236,24 @@ mod tests { engine.runtime.set_arm(id, "tm-1", ArmState::armed()); } - fake.say("pc-1", "ctx:63% -"); + fake.say( + "pc-1", "ctx:63% +", + ); engine.runtime.mark_dirty("pc-1"); evaluate_tick(&engine, &host, 0, 1_000).await; tokio::time::sleep(Duration::from_millis(2_000)).await; - assert_eq!(times_sent(&fake, "once only"), 1, "the runs-once rule never fired"); - assert_eq!(times_sent(&fake, "every time"), 1, "the control never fired"); + assert_eq!( + times_sent(&fake, "once only"), + 1, + "the runs-once rule never fired" + ); + assert_eq!( + times_sent(&fake, "every time"), + 1, + "the control never fired" + ); assert_eq!( fake.announced(), vec!["au-once".to_string()], @@ -3345,7 +4298,11 @@ mod tests { !engine.is_live("au-once"), "a failed stamp took the in-memory retirement with it, so the rule can fire again now" ); - assert_eq!(fake.announced(), vec!["au-once".to_string()], "the windows were not told"); + assert_eq!( + fake.announced(), + vec!["au-once".to_string()], + "the windows were not told" + ); let log = log_details(&fake.store); assert!( log.iter().any(|(kind, detail)| kind == "Failed" @@ -3379,7 +4336,10 @@ mod tests { // exactly that window. evaluate_tick(&engine, &host, 0, 1_000).await; if restarted { - fake.leaves.lock().unwrap().insert("tm-1".into(), "pc-2".into()); + fake.leaves + .lock() + .unwrap() + .insert("tm-1".into(), "pc-2".into()); } tokio::time::sleep(Duration::from_millis(2_000)).await; @@ -3409,17 +4369,30 @@ mod tests { // Before any tick, and inside the grace window, nothing is reported missing — §4.5: at t=0 the // live set is empty and session restore has not run. - assert!(!is_missing(&engine, "au-1", "tm-gone"), "reported before the grace elapsed"); + assert!( + !is_missing(&engine, "au-1", "tm-gone"), + "reported before the grace elapsed" + ); targeting_tick(&engine, &host, 1_000); - assert!(!is_missing(&engine, "au-1", "tm-gone"), "the grace window did not hold"); + assert!( + !is_missing(&engine, "au-1", "tm-gone"), + "the grace window did not hold" + ); // Past the grace, the pinned id is not in the roster and is reported — through the payload // first paint actually calls, not through the tick's return value. targeting_tick(&engine, &host, 120_000); - assert!(is_missing(&engine, "au-1", "tm-gone"), "first paint cannot see what the tick found"); + assert!( + is_missing(&engine, "au-1", "tm-gone"), + "first paint cannot see what the tick found" + ); // And it is retracted the moment the terminal comes back, rather than latching. - _fake.leaves.lock().unwrap().insert("tm-gone".into(), "pc-9".into()); + _fake + .leaves + .lock() + .unwrap() + .insert("tm-gone".into(), "pc-9".into()); _fake.roster.lock().unwrap().push(RosterRow { terminal_id: Some("tm-gone".into()), process_id: "pc-9".into(), @@ -3431,7 +4404,10 @@ mod tests { command_lines: Vec::new(), }); targeting_tick(&engine, &host, 130_000); - assert!(!is_missing(&engine, "au-1", "tm-gone"), "dormant, never dead — it came back"); + assert!( + !is_missing(&engine, "au-1", "tm-gone"), + "dormant, never dead — it came back" + ); } fn is_missing(engine: &Arc, rule_id: &str, tm: &str) -> bool { @@ -3469,7 +4445,11 @@ mod tests { evaluate_tick(&engine, &host, 0, 1_000).await; tokio::time::sleep(Duration::from_millis(2_000)).await; - assert_eq!(times_sent(&fake, "bravo speaking"), 1, "the premise: B has fired once"); + assert_eq!( + times_sent(&fake, "bravo speaking"), + 1, + "the premise: B has fired once" + ); // The user flips A off. Nothing about B changed. let mut off = ctx_rule_saying("au-a", "alpha speaking", 1); @@ -3499,7 +4479,10 @@ mod tests { "the decision must be `held`, and be visible as one: {:?}", log_kinds(&fake.store) ); - assert!(!engine.is_live("au-a"), "the premise: A really did leave the live set"); + assert!( + !engine.is_live("au-a"), + "the premise: A really did leave the live set" + ); } // ============================================================================================= @@ -3516,7 +4499,7 @@ mod tests { // The decision has been made and carried; NOW the terminal goes away completely. fake.close("tm-1"); - run_send(engine.clone(), host.clone(), send).await; + run_crossing(engine.clone(), host.clone(), send).await; let rows = log_rows(&fake.store); assert_eq!(rows.len(), 1, "{:?}", rows); @@ -3534,7 +4517,7 @@ mod tests { let (engine, fake, host) = wired(); let send = pending(&engine, &host, "au-1", ArmState::armed(), 1_000); - run_send(engine.clone(), host.clone(), send).await; + run_crossing(engine.clone(), host.clone(), send).await; tokio::time::sleep(Duration::from_millis(1_500)).await; let rows = log_rows(&fake.store); @@ -3552,7 +4535,9 @@ mod tests { async fn a_row_that_sent_nothing_carries_the_name_as_well() { let (engine, fake, host) = wired(); // Fired and still true: `held`, which is a Decision-class row and so not verbose-gated. - engine.runtime.set_arm("au-1", "tm-1", ArmState::Fired { at_ms: 500 }); + engine + .runtime + .set_arm("au-1", "tm-1", ArmState::Fired { at_ms: 500 }); fake.say("pc-1", "ctx:63%\n"); engine.runtime.mark_dirty("pc-1"); @@ -3560,7 +4545,11 @@ mod tests { let rows = log_rows(&fake.store); assert_eq!(rows.len(), 1, "{:?}", rows); - assert_ne!(rows[0].0, "Sent", "the premise: nothing was sent — {:?}", rows); + assert_ne!( + rows[0].0, "Sent", + "the premise: nothing was sent — {:?}", + rows + ); assert_eq!(rows[0].2.as_deref(), Some("codex · core"), "{:?}", rows); assert!(fake.written().is_empty()); } @@ -3585,12 +4574,17 @@ mod tests { cwd: None, command_lines: Vec::new(), }); - fake.leaves.lock().unwrap().insert("tm-2".into(), "pc-2".into()); + fake.leaves + .lock() + .unwrap() + .insert("tm-2".into(), "pc-2".into()); // Both terminals match `All terminals`, and both have fired. targeting_tick(&engine, &host, 1_000); for tm in ["tm-1", "tm-2"] { - engine.runtime.set_arm("au-1", tm, ArmState::Fired { at_ms: 500 }); + engine + .runtime + .set_arm("au-1", tm, ArmState::Fired { at_ms: 500 }); engine.runtime.set_last_eval("au-1", tm, 500); engine.runtime.record_fire("au-1", tm, 500); } @@ -3599,7 +4593,11 @@ mod tests { fake.close("tm-2"); targeting_tick(&engine, &host, 3_000); - assert_eq!(engine.runtime.arm_state("au-1", "tm-2"), ArmState::Unseen, "the stale key survived"); + assert_eq!( + engine.runtime.arm_state("au-1", "tm-2"), + ArmState::Unseen, + "the stale key survived" + ); assert_eq!(engine.runtime.last_eval("au-1", "tm-2"), None); assert_eq!( engine.runtime.fire_record("au-1", "tm-2"), @@ -3635,7 +4633,7 @@ mod tests { engine.stop(); } - run_send(engine.clone(), host.clone(), send).await; + run_crossing(engine.clone(), host.clone(), send).await; assert_eq!( engine.runtime.arm_state("au-1", "tm-1"), @@ -3659,7 +4657,7 @@ mod tests { *fake.write_err.lock().unwrap() = Some("no writer".into()); } - run_send(engine.clone(), host.clone(), send).await; + run_crossing(engine.clone(), host.clone(), send).await; assert_eq!( engine.runtime.arm_state("au-1", "tm-1"), @@ -3685,23 +4683,34 @@ mod tests { // mutation to `tokio::spawn` survived it. `ends_with` can only be satisfied by the characters // immediately before the call — by the call itself. let engine = strip_comments(include_str!("../automation_engine.rs")); - let start = engine.find("pub fn spawn").expect("spawn must exist"); + let start = engine + .find("pub fn spawn") + .expect("spawn must exist"); let body = &engine[start..]; let outer = body.find("spawn({").expect("it must spawn something"); assert!( body[..outer].ends_with("tauri::async_runtime::"), "the OUTER spawn runs from `.setup()`, where no runtime is entered, so it must go \ through Tauri's wrapper; the call reads `{}spawn({{`", - body[..outer].rsplit('\n').next().unwrap_or_default().trim_start() + body[..outer] + .rsplit('\n') + .next() + .unwrap_or_default() + .trim_start() ); let lib = strip_comments(include_str!("../lib.rs")); - let setup_start = - lib.find("spawn_history_flush_task(state.clone());").expect("the setup site"); + let setup_start = lib + .find("spawn_history_flush_task(state.clone());") + .expect("the setup site"); // Windowed by LINES rather than bytes: `strip_comments` drops comment-only lines and leaves // trailing ones, so a multi-byte character landing inside a fixed byte window panics with a // slice error instead of failing with this test's own message. - let setup = lib[setup_start..].lines().take(12).collect::>().join("\n"); + let setup = lib[setup_start..] + .lines() + .take(12) + .collect::>() + .join("\n"); let setup = setup.as_str(); assert!( !setup.contains("tokio::spawn"), @@ -3729,7 +4738,9 @@ mod tests { once.runs_once = true; let (engine, fake, host) = wire(vec![once]); open_second_terminal(&fake); - engine.runtime.set_watched("au-once", ["tm-1".to_string(), "tm-2".to_string()].into()); + engine + .runtime + .set_watched("au-once", ["tm-1".to_string(), "tm-2".to_string()].into()); for tm in ["tm-1", "tm-2"] { engine.runtime.set_arm("au-once", tm, ArmState::armed()); } @@ -3748,7 +4759,10 @@ mod tests { fake.written() ); assert_eq!( - log_rows(&fake.store).iter().filter(|(k, _, _)| k == "Sent").count(), + log_rows(&fake.store) + .iter() + .filter(|(k, _, _)| k == "Sent") + .count(), 1, "and it logged every one of them" ); @@ -3771,7 +4785,9 @@ mod tests { once.runs_once = true; let (engine, fake, host) = wire(vec![once]); open_second_terminal(&fake); - engine.runtime.set_watched("au-once", ["tm-1".to_string(), "tm-2".to_string()].into()); + engine + .runtime + .set_watched("au-once", ["tm-1".to_string(), "tm-2".to_string()].into()); for tm in ["tm-1", "tm-2"] { engine.runtime.set_arm("au-once", tm, ArmState::armed()); } @@ -3797,7 +4813,10 @@ mod tests { fake.written() ); assert_eq!( - log_rows(&fake.store).iter().filter(|(k, _, _)| k == "Sent").count(), + log_rows(&fake.store) + .iter() + .filter(|(k, _, _)| k == "Sent") + .count(), 1, "and logged both of them" ); @@ -3815,7 +4834,9 @@ mod tests { let mut once = ctx_rule_saying("au-once", "once only", 1); once.runs_once = true; let (engine, fake, host) = wire(vec![once]); - engine.runtime.set_watched("au-once", ["tm-1".to_string()].into()); + engine + .runtime + .set_watched("au-once", ["tm-1".to_string()].into()); engine.runtime.set_arm("au-once", "tm-1", ArmState::armed()); *fake.write_err.lock().unwrap() = Some("no writer".into()); @@ -3828,10 +4849,16 @@ mod tests { // so `written()` shows the attempt either way. A `Sent` row is only written after `deliver` // returns `Ok`. let sent_rows = |f: &FakeHost| { - log_rows(&f.store).iter().filter(|(k, _, _)| k == "Sent").count() + log_rows(&f.store) + .iter() + .filter(|(k, _, _)| k == "Sent") + .count() }; assert_eq!(sent_rows(&fake), 0, "the premise: the write was refused"); - assert!(engine.is_live("au-once"), "a failed send must not complete the rule"); + assert!( + engine.is_live("au-once"), + "a failed send must not complete the rule" + ); assert_eq!( engine.runtime.arm_state("au-once", "tm-1"), ArmState::armed(), @@ -3875,12 +4902,18 @@ mod tests { "the premise: nothing was evaluated, so no arm state moved" ); assert!(log_rows(&fake.store).is_empty(), "§4.5: no read, no row"); - assert!(engine.runtime.is_dirty("pc-1"), "the tick spent a signal no pair could read"); + assert!( + engine.runtime.is_dirty("pc-1"), + "the tick spent a signal no pair could read" + ); // The paired positive, so this is not satisfied by never clearing anything. fake.say("pc-1", "ctx:18%\n"); evaluate_tick(&engine, &host, 0, 2_000).await; - assert!(!engine.runtime.is_dirty("pc-1"), "an ordinary read must still spend the signal"); + assert!( + !engine.runtime.is_dirty("pc-1"), + "an ordinary read must still spend the signal" + ); } /// The other half of B-2's fix, and it had no oracle: **only a `runs_once` rule is deduped.** @@ -3894,7 +4927,9 @@ mod tests { async fn a_repeating_rule_still_sends_to_every_terminal_it_watches() { let (engine, fake, host) = wire(vec![ctx_rule_saying("au-many", "every one", 1)]); open_second_terminal(&fake); - engine.runtime.set_watched("au-many", ["tm-1".to_string(), "tm-2".to_string()].into()); + engine + .runtime + .set_watched("au-many", ["tm-1".to_string(), "tm-2".to_string()].into()); for tm in ["tm-1", "tm-2"] { engine.runtime.set_arm("au-many", tm, ArmState::armed()); } @@ -3930,7 +4965,10 @@ mod tests { // The first pass adopts tm-1 and says so. Drained here, so what is asserted below is the // SECOND change and not this one. tokio::time::sleep(Duration::from_millis(TARGETING_TICK_MS / 2)).await; - assert!(engine.take_state_emit(1_000), "the first pass must announce the rule's first leaf"); + assert!( + engine.take_state_emit(1_000), + "the first pass must announce the rule's first leaf" + ); assert!(engine.runtime.watches("au-1", "tm-1")); // A second terminal opens. `All terminals` + `follow_new` adopts it. @@ -3938,7 +4976,10 @@ mod tests { tokio::time::sleep(Duration::from_millis(TARGETING_TICK_MS * 2)).await; engine.stop(); - assert!(engine.runtime.watches("au-1", "tm-2"), "the premise: it was adopted"); + assert!( + engine.runtime.watches("au-1", "tm-2"), + "the premise: it was adopted" + ); assert!( engine.take_state_emit(10_000), "the watch set grew and no window was told; only `missing` was being diffed" @@ -3952,7 +4993,10 @@ mod tests { let (engine, _fake, host) = wired(); let quiet = tokio::spawn(run_targeting(engine.clone(), host.clone())); tokio::time::sleep(Duration::from_millis(TARGETING_TICK_MS / 2)).await; - assert!(engine.take_state_emit(1_000), "the premise: the first pass adopted tm-1"); + assert!( + engine.take_state_emit(1_000), + "the premise: the first pass adopted tm-1" + ); tokio::time::sleep(Duration::from_millis(TARGETING_TICK_MS * 3)).await; engine.stop(); assert!( @@ -3993,7 +5037,11 @@ mod tests { Some(1_000), "the premise: A was due and ran" ); - assert_eq!(engine.runtime.last_eval("au-b", "tm-1"), Some(900), "and B did not"); + assert_eq!( + engine.runtime.last_eval("au-b", "tm-1"), + Some(900), + "and B did not" + ); assert!( engine.runtime.is_dirty("pc-1"), "A spent the flag on B's behalf, and B never sees this output again" @@ -4004,7 +5052,10 @@ mod tests { // for A instead; that is the rule working, not a second bug.) evaluate_tick(&engine, &host, 0, 1_300).await; assert_eq!(engine.runtime.last_eval("au-b", "tm-1"), Some(1_300)); - assert!(!engine.runtime.is_dirty("pc-1"), "and now the flag is genuinely spent"); + assert!( + !engine.runtime.is_dirty("pc-1"), + "and now the flag is genuinely spent" + ); } /// **M-5: `touch_target` had no production caller**, so `automation_targets` held rows only for @@ -4023,8 +5074,14 @@ mod tests { targeting_tick(&engine, &host, 1_000); let rows = fake.store.targets_for("au-1").unwrap(); - let row = rows.iter().find(|r| r.0 == "tm-1").unwrap_or_else(|| panic!("{:?}", rows)); - assert_eq!(row.1, "matched", "a criterion match is never pinned, so nothing else writes it"); + let row = rows + .iter() + .find(|r| r.0 == "tm-1") + .unwrap_or_else(|| panic!("{:?}", rows)); + assert_eq!( + row.1, "matched", + "a criterion match is never pinned, so nothing else writes it" + ); assert_eq!(row.2.as_deref(), Some("codex · core")); assert_eq!(row.3.as_deref(), Some("D:/sources/work")); } @@ -4037,12 +5094,22 @@ mod tests { #[test] fn the_exit_arm_stops_the_engine_before_anything_slow() { let lib = strip_comments(include_str!("../lib.rs")); - let start = lib.find("if let RunEvent::Exit = event {").expect("the Exit arm"); + let start = lib + .find("if let RunEvent::Exit = event {") + .expect("the Exit arm"); let arm = lib[start..].lines().take(25).collect::>().join("\n"); let arm = arm.as_str(); - let stop = arm.find("automations.stop()").expect("Exit must stop the engine"); - for slow in ["flush_all_history(", "shutdown_mcp_server(", "shutdown_fabric("] { - let at = arm.find(slow).unwrap_or_else(|| panic!("{} left the Exit arm", slow)); + let stop = arm + .find("automations.stop()") + .expect("Exit must stop the engine"); + for slow in [ + "flush_all_history(", + "shutdown_mcp_server(", + "shutdown_fabric(", + ] { + let at = arm + .find(slow) + .unwrap_or_else(|| panic!("{} left the Exit arm", slow)); assert!(stop < at, "the loops keep deciding across {}", slow); } } @@ -4078,7 +5145,10 @@ mod tests { evaluate_tick(&engine, &host, 0, 1_000).await; - assert!(lookups.load(Ordering::Relaxed) >= 2, "the premise: both pairs resolved their leaf"); + assert!( + lookups.load(Ordering::Relaxed) >= 2, + "the premise: both pairs resolved their leaf" + ); assert!( engine.runtime.is_dirty("pc-1"), "the tick cleared a signal that arrived after it had finished reading" @@ -4098,8 +5168,13 @@ mod tests { targeting_tick(&engine, &host, 1_000); - let ids: Vec = - fake.store.targets_for("au-1").unwrap().into_iter().map(|r| r.0).collect(); + let ids: Vec = fake + .store + .targets_for("au-1") + .unwrap() + .into_iter() + .map(|r| r.0) + .collect(); assert_eq!( ids, vec!["tm-1".to_string()], @@ -4107,6 +5182,36 @@ mod tests { ); } + /// An exception is a criterion the roster must answer too. The fake deliberately exposes its + /// process-derived command lines only when `roster` was asked for `CommandContains`, mirroring + /// the production scan gate. Reverting the exclusion half of the criteria collection leaves the + /// line empty, so `tm-1` incorrectly remains watched and this test fails. + #[test] + fn targeting_requests_and_uses_command_lines_for_an_exclusion_criterion() { + let mut rule = ctx_rule("au-exclude-claude"); + rule.criterion = Criterion::AllTerminals; + rule.exclude_criterion = Some(Criterion::CommandContains); + rule.exclude_criterion_value = "claude".into(); + let (engine, fake, host) = wire(vec![rule]); + fake.scanned_command_lines + .lock() + .unwrap() + .insert("tm-1".into(), vec!["pwsh.exe -Command claude".into()]); + + let pass = targeting_tick(&engine, &host, 1_000); + + assert!( + fake.last_roster_criteria() + .contains(&Criterion::CommandContains), + "the exclusion must request the process scan that populates command lines" + ); + assert_eq!( + pass.watched.get("au-exclude-claude"), + Some(&HashSet::new()), + "the populated command line must make the exception remove tm-1" + ); + } + /// **`if report.emit` written as `if false` changed nothing any test could see**, in either of /// the two places it was written. The decision now has one implementation, out where a test can /// reach it. @@ -4135,14 +5240,24 @@ mod tests { // inside the very loop that fills `skipped` — so this row pins THIS FUNCTION's contract, that // `emit` is the gate and the list is not consulted when it is closed, rather than a state the // engine reaches. - assert_eq!(refusals_to_announce(&ReloadReport { emit: false, ..refused }), None); + assert_eq!( + refusals_to_announce(&ReloadReport { + emit: false, + ..refused + }), + None + ); // An empty `skipped` with `emit` set: also unreachable from `reload`, and also this function's // contract — an empty list is not `None`, because a caller with a reason to emit and no ids to // name still emits. *(This comment used to justify the row with "a row written by something // else in the same load"; `reload` has no such mechanism.)* assert_eq!( - refusals_to_announce(&ReloadReport { live: 3, skipped: vec![], emit: true }), + refusals_to_announce(&ReloadReport { + live: 3, + skipped: vec![], + emit: true + }), Some(vec![]) ); } @@ -4162,8 +5277,10 @@ mod tests { engine.runtime.set_arm(id, "tm-1", ArmState::armed()); } engine.runtime.set_last_eval("au-timer", "tm-1", 900); - fake.say("pc-1", "ctx:18% -"); + fake.say( + "pc-1", "ctx:18% +", + ); engine.runtime.mark_dirty("pc-1"); evaluate_tick(&engine, &host, 0, 1_000).await; @@ -4207,7 +5324,11 @@ mod tests { evaluate_tick(&engine, &host, 0, 1_000).await; tokio::time::sleep(Duration::from_millis(4_000)).await; - assert_eq!(times_sent(&fake, "first message"), 1, "the premise: both sent"); + assert_eq!( + times_sent(&fake, "first message"), + 1, + "the premise: both sent" + ); assert_eq!(times_sent(&fake, "second message"), 1); // The first send lands one paste-to-submit gap after the decision; the second waits for the @@ -4215,10 +5336,14 @@ mod tests { let gap = crate::automation::send::PASTE_SUBMIT_GAP_MS as i64; let second_landed = 1_000 + gap * 2; assert!( - engine.runtime.is_settling("tm-1", second_landed + ECHO_SETTLE_MS - 1), + engine + .runtime + .is_settling("tm-1", second_landed + ECHO_SETTLE_MS - 1), "the queued send's window had already been running for the length of its wait" ); - assert!(!engine.runtime.is_settling("tm-1", second_landed + ECHO_SETTLE_MS + 1)); + assert!(!engine + .runtime + .is_settling("tm-1", second_landed + ECHO_SETTLE_MS + 1)); } /// A second live terminal, with ids that share no substring with the first (§7.4). @@ -4260,7 +5385,10 @@ mod tests { tokio::time::sleep(Duration::from_millis(2_000)).await; } - let kinds: Vec = log_rows(&fake.store).into_iter().map(|(k, _, _)| k).collect(); + let kinds: Vec = log_rows(&fake.store) + .into_iter() + .map(|(k, _, _)| k) + .collect(); assert_eq!( kinds.iter().filter(|k| *k == "Sent").count(), 1, @@ -4274,7 +5402,12 @@ mod tests { again, and they evict the `sent` row: {:?}", kinds ); - assert_eq!(kinds.len(), 2, "and nothing else was written at all: {:?}", kinds); + assert_eq!( + kinds.len(), + 2, + "and nothing else was written at all: {:?}", + kinds + ); } /// **H-2: `automation:state` is an ARM TRANSITION event** (§7.2), and it fired only from a @@ -4306,18 +5439,29 @@ mod tests { fake.say("pc-1", "ctx:18%\n"); engine.runtime.mark_dirty("pc-1"); evaluate_tick(&engine, &host, 0, 2_500).await; - assert_eq!(fake.states.load(Ordering::Relaxed), 2, "a refused emit was dropped, not deferred"); + assert_eq!( + fake.states.load(Ordering::Relaxed), + 2, + "a refused emit was dropped, not deferred" + ); // Out of the settle window: Fired + false is a real transition, so it is announced. engine.runtime.mark_dirty("pc-1"); evaluate_tick(&engine, &host, 0, 9_000).await; - assert_eq!(engine.runtime.arm_state("au-1", "tm-1"), ArmState::re_armed()); + assert_eq!( + engine.runtime.arm_state("au-1", "tm-1"), + ArmState::re_armed() + ); assert_eq!(fake.states.load(Ordering::Relaxed), 3); // And still 18%: no transition at all now, so nothing to say however long we wait. engine.runtime.mark_dirty("pc-1"); evaluate_tick(&engine, &host, 0, 20_000).await; - assert_eq!(fake.states.load(Ordering::Relaxed), 3, "a repeat is not a transition"); + assert_eq!( + fake.states.load(Ordering::Relaxed), + 3, + "a repeat is not a transition" + ); } /// **H-6: the rule can go away during the queue wait too.** The wait is up to ten seconds; a user @@ -4329,7 +5473,7 @@ mod tests { let send = pending(&engine, &host, "au-1", ArmState::armed(), 1_000); engine.complete_rule("au-1"); - run_send(engine.clone(), host.clone(), send).await; + run_crossing(engine.clone(), host.clone(), send).await; tokio::time::sleep(Duration::from_millis(1_500)).await; assert!(fake.written().is_empty(), "{:?}", fake.written()); @@ -4348,12 +5492,15 @@ mod tests { let (engine, fake, host) = wired(); let send = pending(&engine, &host, "au-1", ArmState::armed(), 1_000); - run_send(engine.clone(), host.clone(), send).await; + run_crossing(engine.clone(), host.clone(), send).await; tokio::time::sleep(Duration::from_millis(1_500)).await; let ids = fake.written_to(); assert!(!ids.is_empty(), "the premise: something was written"); - assert!(ids.iter().all(|id| id == "pc-1"), "a write was addressed by leaf id: {:?}", ids); + assert!( + ids.iter().all(|id| id == "pc-1"), + "a write was addressed by leaf id: {:?}", + ids + ); } - } diff --git a/src-tauri/src/automation_engine/test_host.rs b/src-tauri/src/automation_engine/test_host.rs index 55e164ae..aa701e4f 100644 --- a/src-tauri/src/automation_engine/test_host.rs +++ b/src-tauri/src/automation_engine/test_host.rs @@ -31,6 +31,11 @@ pub(crate) struct FakeHost { pub(crate) store: Arc, pub(crate) leaves: Mutex>, pub(crate) roster: Mutex>, + /// Process-derived command lines, made visible only when the caller asks the roster for the + /// criterion that requires a process scan. This gives targeting tests the same request/populate + /// seam as `AppState::roster` without taking a real machine snapshot. + pub(crate) scanned_command_lines: Mutex>>, + pub(crate) roster_criteria: Mutex>>, pub(crate) text: Mutex>, /// Every `tail` the engine asked for, in order — **the screen reads, recorded**. /// @@ -67,6 +72,8 @@ impl FakeHost { store: Arc::new(AutomationStore::new_in_memory()), leaves: Mutex::new(HashMap::new()), roster: Mutex::new(Vec::new()), + scanned_command_lines: Mutex::new(HashMap::new()), + roster_criteria: Mutex::new(Vec::new()), text: Mutex::new(HashMap::new()), tails: Mutex::new(Vec::new()), writes: Mutex::new(Vec::new()), @@ -136,6 +143,10 @@ impl FakeHost { pub(crate) fn announced(&self) -> Vec { self.changed.lock().unwrap().iter().flatten().cloned().collect() } + + pub(crate) fn last_roster_criteria(&self) -> Vec { + self.roster_criteria.lock().unwrap().last().cloned().unwrap_or_default() + } } impl EngineHost for FakeHost { @@ -146,13 +157,27 @@ impl EngineHost for FakeHost { } self.leaves.lock().unwrap().get(tm).cloned() } - fn roster(&self, _criteria: &[Criterion]) -> Vec { - self.roster.lock().unwrap().clone() + fn roster(&self, criteria: &[Criterion]) -> Vec { + self.roster_criteria.lock().unwrap().push(criteria.to_vec()); + let mut rows = self.roster.lock().unwrap().clone(); + if criteria.contains(&Criterion::CommandContains) { + let scanned = self.scanned_command_lines.lock().unwrap(); + for row in &mut rows { + if let Some(lines) = row + .terminal_id + .as_deref() + .and_then(|terminal_id| scanned.get(terminal_id)) + { + row.command_lines = lines.clone(); + } + } + } + rows } fn live_processes(&self) -> Vec { self.leaves.lock().unwrap().values().cloned().collect() } - fn tail(&self, pc: &str, _depth: ReadDepth) -> Option { + fn tail(&self, pc: &str, _depth: ReadDepth, _skip_typed_line: bool) -> Option { // Recorded BEFORE the lookup, so a process with no text still counts as a read attempt — // §4.5's dormant terminal is a `None` here, and a path that reaches this port has read. self.tails.lock().unwrap().push(pc.to_string()); @@ -205,6 +230,9 @@ pub(crate) fn ctx_rule(id: &str) -> AutomationRule { criterion_value: String::new(), follow_new: true, target_ids: vec![], + excluded_ids: vec![], + exclude_criterion: None, + exclude_criterion_value: String::new(), completed_at: None, verbose_until: None, sort_order: 1, @@ -212,7 +240,7 @@ pub(crate) fn ctx_rule(id: &str) -> AutomationRule { graph: AutomationGraph { layout: None, timer: None, - monitor: Some(MonitorStep { read: ReadMode::NewOutput, cadence: Cadence::OnOutput, every_ms: 0 }), + monitor: Some(MonitorStep { read: ReadMode::NewOutput, cadence: Cadence::OnOutput, every_ms: 0, skip_typed_line: false }), parse: Some(ParseStep { preset: ParsePreset::Custom, literal: None, @@ -220,13 +248,14 @@ pub(crate) fn ctx_rule(id: &str) -> AutomationRule { keep: Keep::Brackets, }), cond: Some(CondStep { finds: Finds::Reading, op: Some(CompareOp::Gt), threshold: Some(25.0), ..Default::default() }), - action: ActionStep { + action: Some(ActionStep { message: "prepare to do context-hand-off".into(), send_to: SendTo::Matched, submit: true, cli_type: "claude".into(), substitute: false, - }, + }), + webhook: None, }, created_at: 1_000, updated_at: 1_000, @@ -310,7 +339,7 @@ pub(crate) fn log_kinds(store: &AutomationStore) -> Vec { /// the write log by their contents rather than by the order they happen to arrive in. pub(crate) fn ctx_rule_saying(id: &str, message: &str, sort_order: i64) -> AutomationRule { let mut rule = ctx_rule(id); - rule.graph.action.message = message.into(); + rule.graph.action_mut().message = message.into(); rule.sort_order = sort_order; rule } @@ -331,7 +360,7 @@ pub(crate) fn schedule_only_rule(id: &str) -> AutomationRule { rule.graph.timer = Some(TimerStep { mode: TimerMode::DailyAt { minute_of_day: 9 * 60, days: 0b0001_1111 }, }); - rule.graph.action.message = "stand-up notes?".into(); + rule.graph.action_mut().message = "stand-up notes?".into(); rule } diff --git a/src-tauri/src/automation_store.rs b/src-tauri/src/automation_store.rs index a963ca2f..b463932c 100644 --- a/src-tauri/src/automation_store.rs +++ b/src-tauri/src/automation_store.rs @@ -28,9 +28,9 @@ use rusqlite::Connection; /// silently reinterpreting a graph we do not understand is how a rule starts typing the wrong thing /// into a terminal. `reload` logs exactly one entry per skipped rule per load. Plan §7.3. /// -/// `2` as of plan 032 §3.2 (task 27): a rule is stamped `2` only when [`schema_version_for`] finds it -/// actually uses a v2 feature, never merely because this build can write one. -pub const SUPPORTED_SCHEMA_VERSION: i64 = 2; +/// `3` as of the webhook milestone: a rule is stamped only for the newest feature it actually uses, +/// never merely because this build can write one. +pub const SUPPORTED_SCHEMA_VERSION: i64 = 3; // --------------------------------------------------------------------------------------------- // The DTO. These serde names are THE AUTHORITY for the whole feature (plan §7.7): the renderer's @@ -270,6 +270,28 @@ pub struct MonitorStep { pub cadence: Cadence, /// Only meaningful for `Cadence::Timer`. pub every_ms: i64, + /// Drop the logical line the cursor sits on, so a command still being TYPED cannot fire the + /// rule. Reported: a rule matching `deploy` fired the moment the word appeared under the + /// user's fingers, before Enter — the screen genuinely contains the text either way, and the + /// engine has no other signal that separates an echoed keystroke from output. + /// + /// **Opt-in, and it has to be.** The cursor's line is real output for plenty of terminals — a + /// full-screen TUI parks the cursor wherever it likes, and a rule reading the row it happens + /// to rest on would silently lose its match. Off is what every rule written before this field + /// did, and what every rule that never ticks the box keeps doing. + /// + /// **Only that one logical line**, never "the cursor's line and everything below": the + /// reported use is an agentic CLI whose status line sits UNDER the input box, which is the + /// text the rule is watching for. Dropping the tail of the screen would take the value with + /// the noise. + #[serde(default, skip_serializing_if = "is_off")] + pub skip_typed_line: bool, +} + +/// Off is the default, so it is not written — a rule that never ticks the box keeps a blob an +/// older build decodes byte for byte. Same reason as `is_default_join`. +fn is_off(flag: &bool) -> bool { + !*flag } /// Step 2 — "Read a value". Plan §2.2b, §6.4b. @@ -358,6 +380,29 @@ pub struct ActionStep { pub substitute: bool, } +#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +#[serde(rename_all = "camelCase")] +pub enum WebhookProvider { Discord, Teams, Slack, Custom } + +/// An optional destination outside the terminal. Its URL is persisted and sent over IPC in the +/// clear by design, but is never displayed, logged, exported, or put in an error. +#[derive(Clone, PartialEq, serde::Serialize, serde::Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct WebhookStep { + pub provider: WebhookProvider, + pub url: String, + pub body: String, + #[serde(default)] + pub substitute: bool, +} + +impl std::fmt::Debug for WebhookStep { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("WebhookStep").field("provider", &self.provider).field("url", &"") + .field("body", &self.body).field("substitute", &self.substitute).finish() + } +} + fn default_send_to() -> SendTo { SendTo::Matched } @@ -468,7 +513,10 @@ pub struct AutomationGraph { /// shape and this attribute becomes load-bearing rather than decorative. #[serde(default)] pub timer: Option, - pub action: ActionStep, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub action: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub webhook: Option, /// Where the editor's four cards sit on its canvas. /// /// **View state, deliberately inside the rule's blob.** The plan originally kept the layout out @@ -503,14 +551,22 @@ pub struct AutomationGraph { /// opposite (monotonic, "once v2 always v2") is the more obvious thing to write by accident, which /// is why `schema_version_is_stamped_from_what_the_rule_actually_uses` pins the non-sticky case /// explicitly rather than leaving it to be implied by the others (task 27 ruling R4). -pub fn schema_version_for(graph: &AutomationGraph) -> i64 { +pub fn schema_version_for(rule: &AutomationRule) -> i64 { + let graph = &rule.graph; let uses_a_v2_feature = graph.timer.is_some() || graph.monitor.is_none() || graph.parse.is_none() || graph.cond.is_none() || graph.cond.as_ref().is_some_and(|c| !c.clauses.is_empty()) - || graph.action.substitute; - if uses_a_v2_feature { + || graph.action.as_ref().is_some_and(|action| action.substitute); + let uses_a_v3_feature = graph.webhook.is_some() || graph.action.is_none() + || !rule.excluded_ids.is_empty() || rule.exclude_criterion.is_some() + || !rule.exclude_criterion_value.is_empty() + // Ships in this same milestone. An older build decodes the rule fine and ignores the key, + // which is the case the stamp exists to signal: it would read the line the user is still + // typing and fire on it — the exact behaviour the box was ticked to stop. + || graph.monitor.as_ref().is_some_and(|monitor| monitor.skip_typed_line); + if uses_a_v3_feature { 3 } else if uses_a_v2_feature { 2 } else { 1 @@ -542,6 +598,10 @@ impl AutomationGraph { self.cond.as_mut().expect("this fixture's rule has a cond step") } #[track_caller] + pub fn action_mut(&mut self) -> &mut ActionStep { + self.action.as_mut().expect("this fixture's rule has an action step") + } + #[track_caller] pub fn monitor_ref(&self) -> &MonitorStep { self.monitor.as_ref().expect("this fixture's rule has a monitor step") } @@ -592,6 +652,12 @@ pub struct AutomationRule { /// a rule saved across a restart would point at nothing. Plan §7.4. #[serde(default)] pub target_ids: Vec, + #[serde(default)] + pub excluded_ids: Vec, + #[serde(default)] + pub exclude_criterion: Option, + #[serde(default)] + pub exclude_criterion_value: String, // --- runtime flags that outlive a process --- /// Set when a `runs_once` rule fires. `None` means it can still run. @@ -744,8 +810,12 @@ const LAST_SEEN_THROTTLE_MS: i64 = 5 * 60 * 1000; const PINNED_TARGET_IDS_SQL: &str = "SELECT terminal_id FROM automation_targets \ WHERE rule_id = ?1 AND source = 'pinned' ORDER BY added_at, terminal_id"; +const EXCLUDED_TARGET_IDS_SQL: &str = "SELECT terminal_id FROM automation_exclusions \ + WHERE rule_id = ?1 ORDER BY added_at, terminal_id"; + const RULE_COLUMNS: &str ="id, name, enabled, runs_once, target_mode, criterion, criterion_value, \ - follow_new, completed_at, verbose_until, sort_order, schema_version, graph, created_at, updated_at"; + exclude_criterion, exclude_criterion_value, follow_new, completed_at, verbose_until, sort_order, \ + schema_version, graph, created_at, updated_at"; /// Whether an entry is subject to the verbose gate. **Derived from `kind` inside `append`, never /// passed in.** A caller that could label its own entry could gate a `Sent` behind the verbose flag @@ -812,6 +882,8 @@ struct RawRule { target_mode: String, criterion: String, criterion_value: String, + exclude_criterion: Option, + exclude_criterion_value: Option, follow_new: bool, completed_at: Option, verbose_until: Option, @@ -831,24 +903,29 @@ fn read_rule_row(r: &rusqlite::Row<'_>) -> rusqlite::Result { target_mode: r.get(4)?, criterion: r.get(5)?, criterion_value: r.get(6)?, - follow_new: r.get(7)?, - completed_at: r.get(8)?, - verbose_until: r.get(9)?, - sort_order: r.get(10)?, - schema_version: r.get(11)?, - graph: r.get(12)?, - created_at: r.get(13)?, - updated_at: r.get(14)?, + exclude_criterion: r.get(7)?, + exclude_criterion_value: r.get(8)?, + follow_new: r.get(9)?, + completed_at: r.get(10)?, + verbose_until: r.get(11)?, + sort_order: r.get(12)?, + schema_version: r.get(13)?, + graph: r.get(14)?, + created_at: r.get(15)?, + updated_at: r.get(16)?, }) } fn hydrate_rule(raw: RawRule) -> Result { Ok(AutomationRule { - graph: serde_json::from_str(&raw.graph).map_err(|e| { - AutomationStoreError::Invalid(format!("rule {}: bad graph blob: {e}", raw.id)) - })?, + // Serde can quote the malformed value in its error, and a graph contains the webhook URL. + // Keep the decode boundary opaque: list_rules turns this into a skipped-row reason and + // reload persists that reason to the activity log. + graph: serde_json::from_str(&raw.graph) + .map_err(|_| AutomationStoreError::Invalid(format!("rule {}: bad graph blob", raw.id)))?, target_mode: enum_from_db(&raw.target_mode)?, criterion: enum_from_db(&raw.criterion)?, + exclude_criterion: raw.exclude_criterion.as_deref().map(enum_from_db).transpose()?, id: raw.id, name: raw.name, enabled: raw.enabled, @@ -856,6 +933,8 @@ fn hydrate_rule(raw: RawRule) -> Result { criterion_value: raw.criterion_value, follow_new: raw.follow_new, target_ids: Vec::new(), + excluded_ids: Vec::new(), + exclude_criterion_value: raw.exclude_criterion_value.unwrap_or_default(), completed_at: raw.completed_at, verbose_until: raw.verbose_until, sort_order: raw.sort_order, @@ -986,6 +1065,25 @@ impl AutomationStore { store } + /// Insert a graph which could only have arrived from an older or corrupt store. + /// + /// This bypasses serialisation deliberately: tests need the real decode path to see whether a + /// malformed value reaches a user-facing skipped-row reason. + #[cfg(test)] + pub(crate) fn insert_raw_graph_for_test(&self, id: &str, graph: &str) { + let guard = self.conn.lock().unwrap(); + let conn = guard.as_ref().expect("in-memory store is connected"); + conn.execute( + "INSERT INTO automation_rules + (id, name, enabled, runs_once, target_mode, criterion, criterion_value, + follow_new, completed_at, verbose_until, sort_order, schema_version, + graph, created_at, updated_at) + VALUES (?1, 'bad', 1, 0, 'rule', 'allTerminals', '', 1, NULL, NULL, 2, 1, ?2, 1000, 1000)", + rusqlite::params![id, graph], + ) + .expect("insert raw graph"); + } + fn schema(conn: &Connection) -> rusqlite::Result<()> { conn.execute( "CREATE TABLE IF NOT EXISTS automation_rules ( @@ -996,6 +1094,8 @@ impl AutomationStore { target_mode TEXT NOT NULL, criterion TEXT NOT NULL, criterion_value TEXT NOT NULL, + exclude_criterion TEXT, + exclude_criterion_value TEXT, follow_new INTEGER NOT NULL, completed_at INTEGER, verbose_until INTEGER, @@ -1025,6 +1125,15 @@ impl AutomationStore { )", [], )?; + conn.execute( + "CREATE TABLE IF NOT EXISTS automation_exclusions ( + rule_id TEXT NOT NULL, + terminal_id TEXT NOT NULL, + added_at INTEGER NOT NULL, + PRIMARY KEY (rule_id, terminal_id) + )", + [], + )?; // Ordered by `id`, never by `at`: two entries can share a millisecond (verbose mode writes // several terminals per tick) and the wall clock can move backwards after an NTP correction or // a resume, which this app already handles as an event. AUTOINCREMENT rather than a bare @@ -1052,6 +1161,8 @@ impl AutomationStore { // build of this branch keeps its old `automation_targets` and every SELECT naming `folder` // fails against it. Plan §3.4. Self::ensure_column(conn, "automation_targets", "folder", "TEXT")?; + Self::ensure_column(conn, "automation_rules", "exclude_criterion", "TEXT")?; + Self::ensure_column(conn, "automation_rules", "exclude_criterion_value", "TEXT")?; Ok(()) } @@ -1113,6 +1224,19 @@ impl AutomationStore { } } + let mut exclusions: HashMap> = HashMap::new(); + { + let mut stmt = conn.prepare( + "SELECT rule_id, terminal_id FROM automation_exclusions ORDER BY added_at, terminal_id", + )?; + let rows = + stmt.query_map([], |r| Ok((r.get::<_, String>(0)?, r.get::<_, String>(1)?)))?; + for row in rows { + let (rule_id, terminal_id) = row?; + exclusions.entry(rule_id).or_default().push(terminal_id); + } + } + let mut stmt = conn.prepare(&format!( "SELECT {RULE_COLUMNS} FROM automation_rules ORDER BY sort_order, id" ))?; @@ -1124,6 +1248,7 @@ impl AutomationStore { match hydrate_rule(raw) { Ok(mut rule) => { rule.target_ids = targets.remove(&rule.id).unwrap_or_default(); + rule.excluded_ids = exclusions.remove(&rule.id).unwrap_or_default(); out.push(rule); } // §3.3: a row this build cannot decode is ONE rule that does not run, never @@ -1497,6 +1622,13 @@ impl AutomationStore { ids.push(row?); } rule.target_ids = ids; + let mut stmt = conn.prepare(EXCLUDED_TARGET_IDS_SQL)?; + let rows = stmt.query_map([id], |r| r.get::<_, String>(0))?; + let mut ids = Vec::new(); + for row in rows { + ids.push(row?); + } + rule.excluded_ids = ids; Ok(Some(rule)) } } @@ -1600,8 +1732,9 @@ impl AutomationStore { .map_err(|e| AutomationStoreError::Invalid(format!("graph is not serialisable: {e}")))?; let target_mode = enum_to_db(&rule.target_mode)?; let criterion = enum_to_db(&rule.criterion)?; + let exclude_criterion = rule.exclude_criterion.as_ref().map(enum_to_db).transpose()?; let schema_version = if rule.schema_version <= SUPPORTED_SCHEMA_VERSION { - schema_version_for(&rule.graph) + schema_version_for(rule) } else { rule.schema_version }; @@ -1614,9 +1747,10 @@ impl AutomationStore { tx.execute( "INSERT INTO automation_rules ( - id, name, enabled, runs_once, target_mode, criterion, criterion_value, follow_new, - completed_at, verbose_until, sort_order, schema_version, graph, created_at, updated_at) - VALUES (?1,?2,?3,?4,?5,?6,?7,?8,?9,?10,?11,?12,?13,?14,?15) + id, name, enabled, runs_once, target_mode, criterion, criterion_value, + exclude_criterion, exclude_criterion_value, follow_new, completed_at, verbose_until, + sort_order, schema_version, graph, created_at, updated_at) + VALUES (?1,?2,?3,?4,?5,?6,?7,?8,?9,?10,?11,?12,?13,?14,?15,?16,?17) ON CONFLICT(id) DO UPDATE SET name = excluded.name, enabled = excluded.enabled, @@ -1624,6 +1758,8 @@ impl AutomationStore { target_mode = excluded.target_mode, criterion = excluded.criterion, criterion_value = excluded.criterion_value, + exclude_criterion = excluded.exclude_criterion, + exclude_criterion_value = excluded.exclude_criterion_value, follow_new = excluded.follow_new, completed_at = excluded.completed_at, verbose_until = excluded.verbose_until, @@ -1639,6 +1775,8 @@ impl AutomationStore { target_mode, criterion, rule.criterion_value, + exclude_criterion, + rule.exclude_criterion_value, rule.follow_new, rule.completed_at, rule.verbose_until, @@ -1693,6 +1831,17 @@ impl AutomationStore { rusqlite::params![rule.id, id], )?; } + tx.execute( + "DELETE FROM automation_exclusions WHERE rule_id = ?1", + [&rule.id], + )?; + for id in &rule.excluded_ids { + tx.execute( + "INSERT OR IGNORE INTO automation_exclusions (rule_id, terminal_id, added_at) + VALUES (?1, ?2, ?3)", + rusqlite::params![rule.id, id, rule.updated_at], + )?; + } Ok(previous) } @@ -1703,6 +1852,7 @@ impl AutomationStore { let conn = guard.as_mut().ok_or(AutomationStoreError::Disabled)?; let tx = conn.transaction()?; tx.execute("DELETE FROM automation_targets WHERE rule_id = ?1", [id])?; + tx.execute("DELETE FROM automation_exclusions WHERE rule_id = ?1", [id])?; tx.execute("DELETE FROM automation_log WHERE rule_id = ?1", [id])?; let n = tx.execute("DELETE FROM automation_rules WHERE id = ?1", [id])?; tx.commit()?; @@ -2294,6 +2444,7 @@ mod tests { read: ReadMode::NewOutput, cadence: Cadence::OnOutput, every_ms: 30_000, + skip_typed_line: false, }), parse: Some(ParseStep { preset: ParsePreset::Percentage, @@ -2307,13 +2458,14 @@ mod tests { threshold: Some(25.0), ..Default::default() }), - action: ActionStep { + action: Some(ActionStep { message: "prepare to do context-hand-off".to_string(), send_to: SendTo::Matched, submit: true, cli_type: "claude".to_string(), substitute: false, - }, + }), + webhook: None, } } @@ -2330,6 +2482,9 @@ mod tests { // A PINNED rule with no targets is one the enable gate refuses, so a fixture that had // none was every store test arranging a row the product cannot produce. target_ids: vec!["tm-1".to_string()], + excluded_ids: vec![], + exclude_criterion: None, + exclude_criterion_value: String::new(), completed_at: None, verbose_until: None, sort_order: 1, @@ -2340,20 +2495,69 @@ mod tests { } } + #[test] + fn exclusions_round_trip_through_save_and_list() { + let store = AutomationStore::new_in_memory(); + let mut r = rule("au-x"); + r.target_mode = TargetMode::Rule; + r.criterion = Criterion::CommandContains; + r.criterion_value = "claude".into(); + r.excluded_ids = vec!["tm-b".into(), "tm-c".into()]; + r.exclude_criterion = Some(Criterion::WorkingFolderUnder); + r.exclude_criterion_value = "~/scratch".into(); + store.save_rule(&r).unwrap(); + + let back = store.list_rules().unwrap().into_iter().find(|x| x.id == "au-x").unwrap(); + assert_eq!(back.excluded_ids, vec!["tm-b".to_string(), "tm-c".to_string()]); + assert_eq!(back.exclude_criterion, Some(Criterion::WorkingFolderUnder)); + assert_eq!(back.exclude_criterion_value, "~/scratch"); + } + + /// A terminal can be BOTH a pick and an exclusion, and one must not erase the other. This is the + /// test that fails if exclusions are squeezed into `automation_targets.source`, whose primary key + /// (`:1024`) has no room for two memberships of one pair. + #[test] + fn a_terminal_can_be_both_pinned_and_excluded_without_either_erasing_the_other() { + let store = AutomationStore::new_in_memory(); + let mut r = rule("au-x"); + r.target_ids = vec!["tm-a".into(), "tm-b".into()]; // picks + r.excluded_ids = vec!["tm-b".into()]; // and tm-b is also excluded + store.save_rule(&r).unwrap(); + + let back = store.list_rules().unwrap().into_iter().find(|x| x.id == "au-x").unwrap(); + assert_eq!(back.target_ids, vec!["tm-a".to_string(), "tm-b".to_string()]); + assert_eq!(back.excluded_ids, vec!["tm-b".to_string()]); + } + + /// BOTH loaders, not just the bulk one. `read_rule_on` is the path get_rule, duplicate, enable and + /// every target mutation take; if only `list_rules` learns exclusions, a duplicate silently drops + /// them and a target edit writes the rule back without them. + #[test] + fn the_single_rule_loader_returns_exclusions_too() { + let store = AutomationStore::new_in_memory(); + let mut r = rule("au-x"); + r.excluded_ids = vec!["tm-b".into()]; + store.save_rule(&r).unwrap(); + + let one = store.get_rule("au-x").unwrap().unwrap(); + assert_eq!(one.excluded_ids, vec!["tm-b".to_string()], "get_rule must agree with list_rules"); + } + + /// Exclusions must survive a mutation that rewrites the rule for an unrelated reason. + #[test] + fn a_target_mutation_preserves_exclusions() { + let store = AutomationStore::new_in_memory(); + let mut r = rule("au-x"); + r.excluded_ids = vec!["tm-b".into()]; + store.save_rule(&r).unwrap(); + store.add_target_to_rule("au-x", "tm-z", 2_000).unwrap(); + assert_eq!(store.get_rule("au-x").unwrap().unwrap().excluded_ids, vec!["tm-b".to_string()]); + } + /// Insert a row whose `graph` column is arbitrary text, bypassing `save_rule`'s /// serialisation. There is no other way to author a row this build cannot decode. fn write_raw_graph(store: &AutomationStore, id: &str, graph: &str) { - let guard = store.conn.lock().unwrap(); - let conn = guard.as_ref().unwrap(); - conn.execute( - "INSERT INTO automation_rules - (id, name, enabled, runs_once, target_mode, criterion, criterion_value, - follow_new, completed_at, verbose_until, sort_order, schema_version, - graph, created_at, updated_at) - VALUES (?1, 'bad', 1, 0, 'rule', 'allTerminals', '', 1, NULL, NULL, 2, 1, ?2, 1000, 1000)", - rusqlite::params![id, graph], - ) - .unwrap(); + store.insert_raw_graph_for_test(id, graph); } fn rule_named(name: &str) -> AutomationRule { @@ -2588,38 +2792,76 @@ mod tests { fn graph_with_substitute() -> AutomationGraph { let mut g = graph(); - g.action.substitute = true; + g.action_mut().substitute = true; g } + fn graph_with_skip_typed_line() -> AutomationGraph { + let mut g = graph(); + g.monitor_mut().skip_typed_line = true; + g + } + + fn stamped(graph: AutomationGraph) -> i64 { + let mut rule = rule("au-stamp"); + rule.graph = graph; + schema_version_for(&rule) + } + /// One row per term of the predicate, plus the plain rule that must STAY v1. A single row would /// leave the other six silently wrong — `schema_version_for` folds seven conditions into one /// bool, and a table is the only shape that shows a wrong term rather than just a wrong result. #[test] fn schema_version_is_stamped_from_what_the_rule_actually_uses() { assert_eq!( - schema_version_for(&graph()), + stamped(graph()), 1, "a plain four-step rule must still load on an older build" ); - assert_eq!(schema_version_for(&graph_with_timer()), 2); - assert_eq!(schema_version_for(&graph_with_no_monitor()), 2); - assert_eq!(schema_version_for(&graph_with_no_parse()), 2); - assert_eq!(schema_version_for(&graph_with_no_cond()), 2); - assert_eq!(schema_version_for(&graph_with_one_clause()), 2); - assert_eq!(schema_version_for(&graph_with_substitute()), 2); + assert_eq!(stamped(graph_with_timer()), 2); + assert_eq!(stamped(graph_with_no_monitor()), 2); + assert_eq!(stamped(graph_with_no_parse()), 2); + assert_eq!(stamped(graph_with_no_cond()), 2); + assert_eq!(stamped(graph_with_one_clause()), 2); + assert_eq!(stamped(graph_with_substitute()), 2); + let mut webhook = graph(); + webhook.webhook = Some(WebhookStep { provider: WebhookProvider::Discord, url: "https://example.invalid/hook".into(), body: "build failed".into(), substitute: false }); + assert_eq!(stamped(webhook.clone()), 3); + webhook.webhook = None; + assert_eq!(stamped(webhook), 1, "removing the last webhook drops the stamp back"); + let mut no_action = graph(); + no_action.action = None; + assert_eq!(stamped(no_action), 3); + // Ships in the same milestone as the two above. An older build decodes such a rule + // perfectly and ignores the key — it reads the line the user is still typing and fires on + // it, which is precisely what ticking the box was meant to stop. + assert_eq!(stamped(graph_with_skip_typed_line()), 3); // R4: not sticky. Dropping the last clause must not leave the rule permanently v2 — the // opposite (monotonic) behaviour is the more obvious thing to write by accident. let mut g = graph_with_one_clause(); g.cond.as_mut().unwrap().clauses.clear(); assert_eq!( - schema_version_for(&g), + stamped(g), 1, "dropping the last clause makes the rule v1-compatible again" ); } + #[test] + fn a_rule_with_exclusions_is_stamped_v3_even_on_a_v1_graph() { + let mut excluded = rule("au-excluded"); + excluded.schema_version = 1; + excluded.excluded_ids = vec!["tm-secret".into()]; + assert_eq!(schema_version_for(&excluded), 3); + excluded.excluded_ids.clear(); + assert_eq!(schema_version_for(&excluded), 1); + excluded.exclude_criterion_value = "scratch".into(); + assert_eq!(schema_version_for(&excluded), 3); + excluded.exclude_criterion_value.clear(); + assert_eq!(schema_version_for(&excluded), 1); + } + /// §3.2's whole point: merely loading and re-saving an old rule must not brick it for a /// downgrade. The in-memory fold (§5.4, `fold_v1_clauses`) gives a v1 numeric rule one clause /// at LOAD, in the engine, on a copy that is never written back — so the row this test reads @@ -2658,7 +2900,26 @@ mod tests { }"#; let decoded: AutomationGraph = serde_json::from_str(raw).expect("a graph missing only a newer optional field must still decode"); - assert!(!decoded.action.substitute, "a rule from before this field existed must load with it off"); + assert!(!decoded.action.as_ref().unwrap().substitute, "a rule from before this field existed must load with it off"); + } + + #[test] + fn a_webhook_only_rule_writes_no_action_key() { + let mut graph = graph(); + graph.action = None; + graph.webhook = Some(WebhookStep { provider: WebhookProvider::Slack, url: "https://example.invalid/hook".into(), body: "build failed".into(), substitute: false }); + assert!(!serde_json::to_string(&graph).unwrap().contains("\"action\"")); + } + + #[test] + fn a_graph_with_no_webhook_writes_no_webhook_key() { + assert!(!serde_json::to_string(&graph()).unwrap().contains("\"webhook\"")); + } + + #[test] + fn a_v1_rule_still_round_trips_byte_for_byte() { + let v1 = r#"{"monitor":{"read":"newOutput","cadence":"onOutput","everyMs":0},"parse":{"preset":"custom","literal":null,"find":"ctx:(\\d+)%","keep":"brackets"},"cond":{"kind":"number","op":"gt","threshold":25.0},"timer":null,"action":{"message":"prepare to do context-hand-off","sendTo":"matched","submit":true,"cliType":"default","substitute":false}}"#; + assert_eq!(serde_json::to_string(&serde_json::from_str::(v1).unwrap()).unwrap(), v1); } // -- §10.14b ------------------------------------------------------------------------------ @@ -2977,7 +3238,7 @@ mod tests { let store = AutomationStore::new_in_memory(); let mut bad = rule("au-1"); bad.enabled = true; - bad.graph.action.message = String::new(); + bad.graph.action_mut().message = String::new(); assert!(store.save_rule_as_of(&bad, 1_000).is_err(), "an enabled rule with no message"); assert!(store.get_rule("au-1").unwrap().is_none(), "and nothing was written"); @@ -3090,6 +3351,27 @@ mod tests { assert!(store.get_rule("au-2").unwrap().is_some()); } + /// Reusing an id is the observable regression: without `DELETE FROM automation_exclusions`, a + /// newly saved rule silently inherits the deleted rule's exception rows. + #[test] + fn deleting_then_reusing_a_rule_id_does_not_restore_old_exclusions() { + let store = AutomationStore::new_in_memory(); + let mut original = rule("au-x"); + original.excluded_ids = vec!["tm-excluded-before-delete".into()]; + store.save_rule(&original).unwrap(); + + assert!(store.delete_rule("au-x").unwrap()); + + let replacement = rule("au-x"); + store.save_rule(&replacement).unwrap(); + let saved = store.get_rule("au-x").unwrap().unwrap(); + assert!( + saved.excluded_ids.is_empty(), + "the replacement must not inherit exclusions from the deleted rule: {:?}", + saved.excluded_ids + ); + } + #[test] fn mark_completed_stamps_the_rule_and_reports_a_missing_one() { let store = AutomationStore::new_in_memory(); @@ -3422,7 +3704,7 @@ mod tests { let mut rule = enableable("au-bad"); rule.enabled = false; rule.target_ids.clear(); - rule.graph.action.message = String::new(); + rule.graph.action_mut().message = String::new(); store.save_rule(&rule).unwrap(); let refused = store.set_enabled_checked("au-bad", true); @@ -3463,7 +3745,7 @@ mod tests { let mut empty = enableable("au-empty"); empty.enabled = false; empty.target_ids.clear(); - empty.graph.action.message = String::new(); + empty.graph.action_mut().message = String::new(); store.save_rule(&empty).unwrap(); store.set_enabled_checked("au-empty", false).unwrap(); @@ -3484,7 +3766,7 @@ mod tests { let mut broken = enableable("au-1"); broken.enabled = true; - broken.graph.action.message = String::new(); + broken.graph.action_mut().message = String::new(); let refused = store.save_rule(&broken); assert!(matches!(refused, Err(AutomationStoreError::Invalid(_))), "{:?}", refused); assert!(store.get_rule("au-1").unwrap().is_none(), "refused, and yet the row was written"); @@ -4108,6 +4390,46 @@ mod tests { ); } + #[test] + fn malformed_webhook_values_never_escape_decode_or_save_errors() { + let secret = "https://hooks.example.invalid/credential-token"; + let malformed = format!( + r#"{{"webhook":{{"provider":"{secret}","url":"{secret}","body":"done"}}}}"# + ); + + // This is the producer, not a hand-written error: serde really does quote the malformed + // provider value, which is why forwarding its Display would leak the URL. + let raw = serde_json::from_str::(&malformed) + .expect_err("a URL is not a webhook provider") + .to_string(); + assert!(raw.contains(secret), "premise: serde produced the secret: {raw}"); + + let store = AutomationStore::new_in_memory(); + write_raw_graph(&store, "au-malformed", &malformed); + assert!(store.list_rules().unwrap().is_empty()); + let skipped = store.take_skipped_rows(); + assert_eq!(skipped.len(), 1); + assert!(!skipped[0].1.contains(secret), "skipped row leaked: {:?}", skipped[0]); + assert!(skipped[0].1.contains("bad graph blob")); + + // Save errors also carry the store's Display through the command layer. Exercise the real + // enable/save validation with a well-typed, secret-bearing webhook rule rather than + // inventing an error text. + let mut invalid = rule("au-save"); + invalid.graph.webhook = Some(WebhookStep { + provider: WebhookProvider::Custom, + url: secret.to_string(), + body: r#"{\"result\": ${value}}"#.to_string(), + substitute: true, + }); + invalid.graph.parse.as_mut().expect("parse fixture").find = r"(?\\w+)".to_string(); + let error = store + .save_rule(&invalid) + .expect_err("post-substitution custom JSON is refused") + .to_string(); + assert!(!error.contains(secret), "save error leaked: {error}"); + } + // ----------------------------------------------------------------------------------------- // Plan 032 §5.2/§5.3 — `finds` (read depth) split from the per-clause `Test` (comparison). // ----------------------------------------------------------------------------------------- @@ -4291,6 +4613,28 @@ mod tests { } } + /// `skipTypedLine` behaves the way every other added field on this wire has to: absent decodes + /// off, off writes nothing, and on survives a round trip. + /// + /// The middle one is the load-bearing clause and the reason this field is + /// `skip_serializing_if` where `substitute` is not. `MonitorStep` is on EVERY watching rule, so + /// a field that always serialised would rewrite every stored blob the first time this build + /// touched it — `a_v1_rule_still_round_trips_byte_for_byte` is the same claim from the other + /// end, and would have caught it as a failure rather than as a decision. + #[test] + fn skip_typed_line_defaults_off_writes_nothing_off_and_round_trips_on() { + let older = r#"{"read":"newOutput","cadence":"onOutput","everyMs":0}"#; + let decoded: MonitorStep = serde_json::from_str(older) + .expect("a monitor step written before this field existed must still decode"); + assert!(!decoded.skip_typed_line, "an older rule must not acquire the opt-in from a default"); + assert_eq!(serde_json::to_string(&decoded).unwrap(), older, "and must not gain the key"); + + let on = MonitorStep { skip_typed_line: true, ..decoded }; + let s = serde_json::to_string(&on).unwrap(); + assert!(s.contains(r#""skipTypedLine":true"#), "written when it is actually used: {s}"); + assert_eq!(serde_json::from_str::(&s).unwrap(), on, "round trip of {s}"); + } + /// The whole reason `timer` is `#[serde(default)]`: a rule saved by a build before this /// milestone has no `timer` key in its graph blob at all, and that older JSON must still /// decode — with `timer: None`, not a decode failure. Mirrors @@ -4327,13 +4671,14 @@ mod tests { timer: Some(TimerStep { mode: TimerMode::DailyAt { minute_of_day: 9 * 60, days: 0b0001_1111 }, }), - action: ActionStep { + action: Some(ActionStep { message: "stand-up notes?".to_string(), send_to: SendTo::Matched, submit: true, cli_type: "default".to_string(), substitute: false, - }, + }), + webhook: None, layout: None, }; diff --git a/src-tauri/src/automation_validation.rs b/src-tauri/src/automation_validation.rs index 1bfa2e11..5ca35fc7 100644 --- a/src-tauri/src/automation_validation.rs +++ b/src-tauri/src/automation_validation.rs @@ -22,12 +22,15 @@ //! about the parse step alone and need nothing but the graph. **M3 lands the whole-rule rules** (no //! terminals, empty message, the echo warning) with the enable path, and **M5 the shared fixture**. +use std::collections::BTreeMap; + use regex::{Regex, RegexBuilder}; +use reqwest::Url; -use crate::automation_engine::subst; +use crate::automation_engine::{eval, subst}; use crate::automation_store::{ AutomationGraph, AutomationRule, Cadence, Criterion, Finds, Keep, ParseStep, Source, TargetMode, - Test, TextOp, TimerMode, TimerStep, MINUTES_PER_DAY, WEEKDAY_BITS_MASK, + Test, TextOp, TimerMode, TimerStep, WebhookProvider, MINUTES_PER_DAY, WEEKDAY_BITS_MASK, }; /// Whether a problem stops the rule running, or merely tells the user something. @@ -129,6 +132,34 @@ fn token_supplied(compiled: &Regex, group: Option, name: Option<&str>) -> } } +fn sample_webhook_captures(compiled: &Regex) -> eval::Captures { + let groups = (0..compiled.captures_len()) + .map(|index| Some(format!("[g{index}]"))) + .collect(); + let named = compiled + .capture_names() + .filter_map(|name| name.map(|name| (name.to_string(), Some(format!("[{name}]"))))) + .collect::>(); + + eval::Captures { groups, named } +} + +fn rendered_webhook_body(webhook: &crate::automation_store::WebhookStep, parse: Option<&ParseStep>) -> String { + if !webhook.substitute { + return webhook.body.clone(); + } + let Some(parse) = parse else { + return webhook.body.clone(); + }; + let Ok(compiled) = compile(&parse.find) else { + return webhook.body.clone(); + }; + match subst::substitute(&webhook.body, Some(&sample_webhook_captures(&compiled))) { + Ok(body) => body, + Err(_) => webhook.body.clone(), + } +} + /// `$0` / `$2` / `${name}`, for a clause's own problem message. /// /// `pub(crate)` rather than a second copy: `dry.rs`'s Test-pane wording for a clause list needs the @@ -298,6 +329,17 @@ pub const MAX_DELAY_MS: i64 = 10 * 60 * 1_000; /// keep them apart: this fires only when `scheduled` is false, and that code fires only when it is /// true. One graph can never trip both with contradictory remedies. /// +/// **A webhook is a DESTINATION, and it used to be read here as a trigger.** The guard exempted +/// any graph carrying one, which does nothing for a webhook-only rule — that shape is already +/// exempt for having no terminal message — and silently exempted the shapes that matter: a rule +/// with a webhook and nothing to start it reported NO problem at all, so it saved, enabled, and +/// never fired. Reachable through the REST API and an import from the day the step existed, and +/// reachable from the editor the moment a card could be deleted: take the three reading steps +/// off a webhook rule with no wait and this is exactly what is left. The two questions are asked +/// separately now — *can anything trigger it* (`has_input_steps || scheduled`) and *is there +/// anything to do* (`has_destination`) — which is what the message underneath has always +/// claimed to be about. +/// /// The remedy differs by shape, and saying so honestly is the whole reason this is not /// `timer.delayWithoutMonitor`: with a Wait step already on the canvas the fix is either add a /// Watch step or switch that Wait to a schedule; with no Wait step at all there is no "switch it" to @@ -316,7 +358,11 @@ pub const MAX_DELAY_MS: i64 = 10 * 60 * 1_000; fn never_runs_problem(graph: &AutomationGraph) -> Option { let has_input_steps = graph.monitor.is_some() && graph.parse.is_some() && graph.cond.is_some(); let scheduled = matches!(graph.timer, Some(TimerStep { mode: TimerMode::DailyAt { .. } })); - if has_input_steps || scheduled || graph.action.message.trim().is_empty() { + let has_terminal_destination = graph.action.as_ref().is_some_and(|action| !action.message.trim().is_empty()); + // **A webhook is a DESTINATION, not a trigger**, and conflating the two is what let an + // unrunnable rule report nothing at all — see this function's own doc. + let has_destination = has_terminal_destination || graph.webhook.is_some(); + if has_input_steps || scheduled || !has_destination { return None; } let message = if graph.timer.is_some() { @@ -517,6 +563,14 @@ in the message, as $2, $3 and so on.", /// worse than one that says so. pub const MIN_TIMER_MS: i64 = crate::automation_engine::due::EVENT_MIN_INTERVAL_MS; +/// Which terminal criteria have a companion value field to fill in. +/// +/// `AllTerminals` is the one selector that deliberately needs no value. Both the watched-set +/// criterion and its optional exclusion use this same predicate so the two controls cannot drift. +fn criterion_needs_value(criterion: Criterion) -> bool { + !matches!(criterion, Criterion::AllTerminals) +} + /// Everything wrong with a WHOLE rule — §6.5's five categories: **target, interval, pattern, /// threshold, message**. /// @@ -547,8 +601,7 @@ pub fn problems(rule: &AutomationRule) -> Vec { } } TargetMode::Rule => { - let needs_value = !matches!(rule.criterion, Criterion::AllTerminals); - if needs_value && rule.criterion_value.trim().is_empty() { + if criterion_needs_value(rule.criterion) && rule.criterion_value.trim().is_empty() { out.push(Problem::new( Severity::Blocks, "targets", @@ -556,6 +609,19 @@ pub fn problems(rule: &AutomationRule) -> Vec { "Fill in what the terminals must match, or watch all terminals instead.", )); } + + if rule + .exclude_criterion + .is_some_and(criterion_needs_value) + && rule.exclude_criterion_value.trim().is_empty() + { + out.push(Problem::new( + Severity::Blocks, + "targets", + "targets.excludeValueEmpty", + "Fill in what the exclusion must match, or exclude all terminals instead.", + )); + } } } @@ -622,14 +688,26 @@ pub fn problems(rule: &AutomationRule) -> Vec { out.extend(timer_problems(&rule.graph)); // --- message -------------------------------------------------------------------------------- - if rule.graph.action.message.trim().is_empty() { + if rule.graph.action.is_none() && rule.graph.webhook.is_none() { + out.push(Problem::new( + Severity::Blocks, + "action", + "rule.noDestination", + "Add a terminal message or a webhook destination.", + )); + } else if rule + .graph + .action + .as_ref() + .is_some_and(|action| action.message.trim().is_empty()) + { out.push(Problem::new( Severity::Blocks, "action", "action.empty", "Enter the message this rule should type.", )); - } else if let Some(parse) = parse_step(&rule.graph) { + } else if let (Some(action), Some(parse)) = (rule.graph.action.as_ref(), parse_step(&rule.graph)) { // §2.6's failure, told to the user before it happens: a rule whose own message matches its // own pattern reads its own echo. The needle guard handles it, which is why this WARNS — // but the guard has a TTL and a cap, and a user who can see the collision can avoid it. @@ -647,7 +725,7 @@ pub fn problems(rule: &AutomationRule) -> Vec { // message `HANDOFF now` — was warned about as an echo of itself. Both mirrors had the same // bug, so the shared fixture agreed with itself and could not see it. if let Ok(re) = compile(&parse.find) { - if re.is_match(&rule.graph.action.message) { + if re.is_match(&action.message) { out.push(Problem::new( Severity::Warns, "action", @@ -672,22 +750,41 @@ pub fn problems(rule: &AutomationRule) -> Vec { // toggle claims the message inserts a capture, and a rule with no parse step at all captures // nothing, exactly like one whose pattern is still empty. `parse_step` is what makes the two // spellings indistinguishable to this check. - if rule.graph.action.substitute { + for (field, message) in [ + rule.graph + .action + .as_ref() + .filter(|action| action.substitute) + .map(|action| ("action", action.message.as_str())), + rule.graph + .webhook + .as_ref() + .filter(|webhook| webhook.substitute) + .map(|webhook| ("webhook", webhook.body.as_str())), + ] + .into_iter() + .flatten() + { match parse_step(&rule.graph) { - // The toggle itself claims the message inserts a capture, which nothing can be true - // of before a pattern exists — asked regardless of whether a token has actually been - // typed yet, the same way the threshold check above is asked regardless of what a - // clause would compare against. - None => out.push(Problem::new( + // **The TOKEN is what claims a capture, not the flag** — and that is a correction. + // This used to fire for a flag-on message whatever it contained, on the ground that + // *"the toggle itself claims the message inserts a capture"*. True while the flag was + // an explicit opt-in a user had to reach for; false the day it became the default, at + // which point every schedule rule — which has no parse step by construction (§6.3) — + // would have opened blocked by a switch nobody touched. A message naming no token + // substitutes to itself: `subst::substitute` returns it unchanged for a `None` capture + // set, so there is nothing to report. + None if !subst::tokens_used(message).is_empty() => out.push(Problem::new( Severity::Blocks, - "action", + field, "action.tokenWithoutParse", "This message inserts captured values, but the rule has no pattern to capture them from.", )), + None => {}, Some(parse) => { if let Ok(compiled) = compile(&parse.find) { let count = compiled.captures_len().saturating_sub(1); - for token in subst::tokens_used(&rule.graph.action.message) { + for token in subst::tokens_used(message) { let bad = match &token { subst::Token::Whole => false, subst::Token::Group(n) => !token_supplied(&compiled, Some(*n), None), @@ -700,7 +797,7 @@ pub fn problems(rule: &AutomationRule) -> Vec { } out.push(Problem::new( Severity::Blocks, - "action", + field, "action.unknownToken", format!( "{token} has nothing to stand for. The pattern in Read a value has \ @@ -714,6 +811,53 @@ pub fn problems(rule: &AutomationRule) -> Vec { } } + // --- webhook --------------------------------------------------------------------------------- + if let Some(webhook) = rule.graph.webhook.as_ref() { + if webhook.url.trim().is_empty() { + out.push(Problem::new( + Severity::Blocks, + "webhook", + "webhook.urlEmpty", + "Provide a webhook URL.", + )); + } else if let Ok(url) = Url::parse(webhook.url.trim()) { + if url.scheme() != "https" { + out.push(Problem::new( + Severity::Blocks, + "webhook", + "webhook.urlNotHttps", + "Provide an https webhook URL.", + )); + } + } else { + out.push(Problem::new( + Severity::Blocks, + "webhook", + "webhook.urlMalformed", + "Provide a well-formed webhook URL.", + )); + } + + if webhook.body.trim().is_empty() { + out.push(Problem::new( + Severity::Blocks, + "webhook", + "webhook.bodyEmpty", + "Enter a webhook body.", + )); + } else if webhook.provider == WebhookProvider::Custom { + let rendered_body = rendered_webhook_body(webhook, parse_step(&rule.graph)); + if serde_json::from_str::(&rendered_body).is_err() { + out.push(Problem::new( + Severity::Blocks, + "webhook", + "webhook.bodyNotJson", + "The webhook body must be valid JSON.", + )); + } + } + } + // STABLE, so within each severity the problems stay in step order — targets, monitor, parse, // cond, timer, action — which is the order the inspector's problem list draws them in. out.sort_by_key(|p| !p.blocks()); @@ -745,16 +889,17 @@ mod tests { AutomationGraph { layout: None, timer: None, - monitor: Some(MonitorStep { read: ReadMode::NewOutput, cadence: Cadence::OnOutput, every_ms: 0 }), + monitor: Some(MonitorStep { read: ReadMode::NewOutput, cadence: Cadence::OnOutput, every_ms: 0, skip_typed_line: false }), parse: Some(ParseStep { preset: ParsePreset::Custom, literal: None, find: find.into(), keep }), cond: Some(CondStep { finds: Finds::Reading, op: Some(CompareOp::Gt), threshold: Some(25.0), ..Default::default() }), - action: ActionStep { + action: Some(ActionStep { message: "m".into(), send_to: SendTo::Matched, submit: true, cli_type: "default".into(), substitute: false, - }, + }), + webhook: None, } } @@ -869,6 +1014,9 @@ mod tests { criterion_value: String::new(), follow_new: true, target_ids: vec!["tm-1".into()], + excluded_ids: vec![], + exclude_criterion: None, + exclude_criterion_value: String::new(), completed_at: None, verbose_until: None, sort_order: 1, @@ -934,7 +1082,7 @@ mod tests { ), ( "nothing to type", - Box::new(|r: &mut AutomationRule| r.graph.action.message = " ".into()), + Box::new(|r: &mut AutomationRule| r.graph.action_mut().message = " ".into()), "action", ), ]; @@ -982,7 +1130,7 @@ mod tests { let mut rule = valid_rule(); rule.graph.parse_mut().find = "HANDOFF".into(); rule.graph.cond = Some(CondStep { finds: Finds::Event, ..Default::default() }); - rule.graph.action.message = "HANDOFF now".into(); + rule.graph.action_mut().message = "HANDOFF now".into(); let found = problems(&rule); assert_eq!(found.len(), 1, "{:?}", found); @@ -1002,7 +1150,7 @@ mod tests { fn blocking_problems_come_before_warnings() { let mut rule = valid_rule(); rule.graph.parse_mut().find = r"ctx:(\d+)(%)".into(); - rule.graph.action.message = String::new(); + rule.graph.action_mut().message = String::new(); let found = problems(&rule); assert_eq!(found.len(), 2, "{:?}", found); @@ -1055,7 +1203,7 @@ mod tests { // about. The count is a floor, not the exact number, so adding a case is not a two-file // edit. assert!( - fixture.cases.len() >= 20, + fixture.cases.len() >= 66, "the shared fixture has shrunk to {} cases", fixture.cases.len() ); @@ -1089,6 +1237,7 @@ mod tests { for code in [ "targets.empty", "targets.criterion", + "targets.excludeValueEmpty", "monitor.interval", "parse.empty", "parse.uncompilable", @@ -1109,6 +1258,12 @@ mod tests { "action.echo", "action.tokenWithoutParse", "action.unknownToken", + "rule.noDestination", + "webhook.urlEmpty", + "webhook.urlMalformed", + "webhook.urlNotHttps", + "webhook.bodyEmpty", + "webhook.bodyNotJson", ] { assert!(codes.contains(code), "no fixture case produces `{code}`"); } @@ -1248,7 +1403,7 @@ mod tests { rule.graph.parse = None; rule.graph.cond = None; rule.graph.timer = Some(TimerStep { mode: TimerMode::AfterMatch { delay_ms: 30_000 } }); - rule.graph.action.message = "resume".into(); + rule.graph.action_mut().message = "resume".into(); let found = problems(&rule); assert_eq!( @@ -1440,6 +1595,9 @@ mod tests { criterion_value: String::new(), follow_new: true, target_ids: vec![], + excluded_ids: vec![], + exclude_criterion: None, + exclude_criterion_value: String::new(), completed_at: None, verbose_until: None, sort_order: 0, @@ -1449,7 +1607,7 @@ mod tests { updated_at: 0, }; rule.graph.cond = Some(CondStep { finds: Finds::Event, ..Default::default() }); - rule.graph.action.message = "anything at all".into(); + rule.graph.action_mut().message = "anything at all".into(); let found = problems(&rule); assert_eq!(found.len(), 1, "only the empty pattern, no echo warning: {found:?}"); diff --git a/src-tauri/src/automation_webhook.rs b/src-tauri/src/automation_webhook.rs new file mode 100644 index 00000000..dbf4b600 --- /dev/null +++ b/src-tauri/src/automation_webhook.rs @@ -0,0 +1,367 @@ +//! Sending an automation's webhook destination. +//! +//! The webhook URL is deliberately confined to the request builder. In particular, no error type +//! in this module retains a `reqwest::Error`: its `Display` and `Debug` implementations can contain +//! the request URL, which may itself contain a credential. + +use std::time::Duration; + +use crate::automation_store::{WebhookProvider, WebhookStep}; + +/// Bound one webhook attempt. A webhook send is dispatched by the engine; this timeout prevents a +/// peer that accepts a connection but never replies from holding that dispatch indefinitely. +const WEBHOOK_TIMEOUT: Duration = Duration::from_secs(10); + +/// The safe-to-render class of a transport failure. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum WebhookTransportClass { + Connect, + Timeout, + Request, +} + +impl std::fmt::Display for WebhookTransportClass { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Connect => f.write_str("connect"), + Self::Timeout => f.write_str("timeout"), + Self::Request => f.write_str("request"), + } + } +} + +/// A webhook failure that is safe to put in an automation activity entry. +/// +/// This intentionally stores only an HTTP status or a coarse transport class. Do not add a +/// `reqwest::Error`, request URL, or response body here: all of those can carry the user's secret +/// endpoint into a display or log surface. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum WebhookError { + Transport(WebhookTransportClass), + Status(reqwest::StatusCode), +} + +impl WebhookError { + fn from_transport(error: reqwest::Error) -> Self { + let class = if error.is_timeout() { + WebhookTransportClass::Timeout + } else if error.is_connect() { + WebhookTransportClass::Connect + } else { + WebhookTransportClass::Request + }; + Self::Transport(class) + } +} + +impl std::fmt::Display for WebhookError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Transport(class) => write!(f, "webhook transport failed ({class})"), + Self::Status(status) => write!(f, "webhook returned status {status}"), + } + } +} + +impl std::error::Error for WebhookError {} + +/// Send a configured webhook body to its destination. +pub async fn send(webhook: &WebhookStep) -> Result<(), WebhookError> { + send_body(webhook, &webhook.body).await +} + +/// Send a resolved webhook body to its destination. +/// +/// Preset providers always receive a newly-created JSON wrapper. `message` is encoded as a JSON +/// value, never interpolated into a JSON string. A Custom endpoint instead receives `message`'s +/// bytes unchanged, including whitespace and duplicate JSON object keys. The engine uses this only +/// after resolving an opted-in capture substitution; normal callers should use [`send`]. +pub async fn send_body(webhook: &WebhookStep, message: &str) -> Result<(), WebhookError> { + let body = payload(webhook.provider, message); + let client = reqwest::Client::builder() + .timeout(WEBHOOK_TIMEOUT) + .build() + .map_err(WebhookError::from_transport)?; + let response = client + .post(&webhook.url) + .header(reqwest::header::CONTENT_TYPE, "application/json") + .body(body) + .send() + .await + .map_err(WebhookError::from_transport)?; + + if response.status().is_success() { + Ok(()) + } else { + Err(WebhookError::Status(response.status())) + } +} + +pub(crate) fn payload(provider: WebhookProvider, message: &str) -> Vec { + match provider { + WebhookProvider::Discord => serde_json::to_vec(&serde_json::json!({ "content": message })) + .expect("a string is always serializable as JSON"), + WebhookProvider::Slack => serde_json::to_vec(&serde_json::json!({ "text": message })) + .expect("a string is always serializable as JSON"), + WebhookProvider::Teams => serde_json::to_vec(&serde_json::json!({ + "@type": "MessageCard", + "@context": "http://schema.org/extensions", + "text": message, + })) + .expect("a string is always serializable as JSON"), + WebhookProvider::Custom => message.as_bytes().to_vec(), + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::automation_store::AutomationGraph; + use std::io::{Read, Write}; + use std::net::{TcpListener, TcpStream}; + use std::sync::mpsc::{self, Receiver}; + + fn webhook(provider: WebhookProvider, url: String, body: &str) -> WebhookStep { + WebhookStep { + provider, + url, + body: body.to_string(), + substitute: false, + } + } + + /// A one-request loopback server. It is deliberately a local listener rather than a mock so + /// the assertions cover the actual reqwest request bytes without contacting any real network. + fn capture_endpoint() -> (String, Receiver>) { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind loopback capture listener"); + let url = format!( + "http://{}", + listener.local_addr().expect("listener address") + ); + let (sent, received) = mpsc::channel(); + std::thread::spawn(move || { + let (mut stream, _) = listener.accept().expect("accept webhook request"); + let request = read_request(&mut stream); + stream + .write_all( + b"HTTP/1.1 204 No Content\r\nContent-Length: 0\r\nConnection: close\r\n\r\n", + ) + .expect("reply to webhook request"); + sent.send(request).expect("return captured request"); + }); + (url, received) + } + + fn read_request(stream: &mut TcpStream) -> Vec { + stream + .set_read_timeout(Some(Duration::from_secs(3))) + .expect("set capture read timeout"); + let mut request = Vec::new(); + let mut buffer = [0_u8; 1024]; + let mut expected = None; + loop { + let read = stream.read(&mut buffer).expect("read webhook request"); + assert_ne!(read, 0, "client closed before completing request"); + request.extend_from_slice(&buffer[..read]); + if expected.is_none() { + expected = content_length(&request).map(|length| { + request + .windows(4) + .position(|bytes| bytes == b"\r\n\r\n") + .expect("headers terminate") + + 4 + + length + }); + } + if expected.is_some_and(|length| request.len() >= length) { + return request; + } + } + } + + fn content_length(request: &[u8]) -> Option { + let header_end = request.windows(4).position(|bytes| bytes == b"\r\n\r\n")?; + let headers = + std::str::from_utf8(&request[..header_end]).expect("request headers are text"); + headers.lines().find_map(|line| { + let (name, value) = line.split_once(':')?; + name.eq_ignore_ascii_case("content-length") + .then(|| value.trim().parse().expect("numeric content length")) + }) + } + + fn captured_body(request: Vec) -> Vec { + let header_end = request + .windows(4) + .position(|bytes| bytes == b"\r\n\r\n") + .expect("headers terminate"); + request[header_end + 4..].to_vec() + } + + fn receive_body(receiver: Receiver>) -> Vec { + captured_body( + receiver + .recv_timeout(Duration::from_secs(3)) + .expect("webhook capture completed"), + ) + } + + #[tokio::test] + async fn discord_wraps_the_message_in_content() { + let (url, captured) = capture_endpoint(); + send(&webhook(WebhookProvider::Discord, url, "build failed")) + .await + .expect("discord webhook succeeds"); + assert_eq!(receive_body(captured), br#"{"content":"build failed"}"#); + } + + #[tokio::test] + async fn slack_wraps_the_message_in_text() { + let (url, captured) = capture_endpoint(); + send(&webhook(WebhookProvider::Slack, url, "build failed")) + .await + .expect("slack webhook succeeds"); + assert_eq!(receive_body(captured), br#"{"text":"build failed"}"#); + } + + #[tokio::test] + async fn teams_uses_a_message_card() { + let (url, captured) = capture_endpoint(); + send(&webhook(WebhookProvider::Teams, url, "build failed")) + .await + .expect("teams webhook succeeds"); + let body: serde_json::Value = + serde_json::from_slice(&receive_body(captured)).expect("teams payload is JSON"); + assert_eq!(body["@type"], "MessageCard"); + assert_eq!(body["text"], "build failed"); + } + + #[tokio::test] + async fn a_custom_endpoint_posts_the_body_verbatim() { + let (url, captured) = capture_endpoint(); + let raw = "{ \"z\" : 1, \"a\" : 2, \"a\" : 3 }\n"; + send(&webhook(WebhookProvider::Custom, url, raw)) + .await + .expect("custom webhook succeeds"); + assert_eq!(receive_body(captured), raw.as_bytes()); + } + + /// The renderer preview has its own implementation, so the exact bytes for every provider live + /// in one fixture both sides read. The sender and dry-run producer share `payload`, so they + /// cannot drift on payload bytes either. + #[test] + fn payload_matches_the_shared_fixture() { + #[derive(serde::Deserialize)] + struct Fixture { + cases: Vec, + } + #[derive(serde::Deserialize)] + struct Case { + name: String, + provider: WebhookProvider, + message: String, + expected: String, + } + + let raw = include_str!( + "../../src/renderer/components/Automation/__fixtures__/webhookPayloadCases.json" + ); + let fixture: Fixture = serde_json::from_str(raw).expect("the shared payload fixture parses"); + assert_eq!(fixture.cases.len(), 7, "all provider and escaping cases stay covered"); + + for case in fixture.cases { + let actual = String::from_utf8(payload(case.provider, &case.message)) + .expect("every payload fixture body is UTF-8"); + assert_eq!(actual, case.expected, "payload fixture case: {}", case.name); + } + } + + #[tokio::test] + async fn a_capture_containing_json_syntax_cannot_break_a_preset_payload() { + let (url, captured) = capture_endpoint(); + let capture = "quote: \"; slash: \\; newline:\n{\"injected\":true}"; + send(&webhook(WebhookProvider::Discord, url, capture)) + .await + .expect("discord webhook succeeds"); + let body: serde_json::Value = + serde_json::from_slice(&receive_body(captured)).expect("preset payload remains JSON"); + assert_eq!(body, serde_json::json!({ "content": capture })); + assert_eq!(body.as_object().expect("object payload").len(), 1); + } + + #[tokio::test] + async fn a_real_transport_error_never_renders_the_url() { + // Reserving then dropping a loopback port makes a genuine connection failure without any + // traffic leaving this machine. The request URL deliberately contains all three values that + // must not reach a display surface. + let reserved = TcpListener::bind("127.0.0.1:0").expect("reserve an unused loopback port"); + let port = reserved.local_addr().expect("reserved address").port(); + drop(reserved); + let url = format!("http://127.0.0.1:{port}/url-leak-path/credential-token"); + let graph = AutomationGraph { + monitor: None, + parse: None, + cond: None, + timer: None, + action: None, + webhook: Some(webhook( + WebhookProvider::Discord, + url.clone(), + "build failed", + )), + layout: None, + }; + let graph_debug = format!("{graph:?}"); + assert!(graph_debug.contains("url: \"\"")); + assert!(!graph_debug.contains(&url)); + let raw = reqwest::Client::builder() + // Do not let a machine-wide proxy turn this local refusal into a real network request. + .no_proxy() + .timeout(Duration::from_secs(1)) + .build() + .expect("build client") + .post(&url) + .send() + .await + .expect_err("dropped loopback port refuses the request"); + + assert_eq!( + raw.url() + .expect("reqwest attaches the request URL to this transport error") + .as_str(), + url, + "this must be a real URL-bearing reqwest error before conversion" + ); + let rendered = WebhookError::from_transport(raw).to_string(); + for secret in [&url, "127.0.0.1", "url-leak-path"] { + assert!( + !rendered.contains(secret), + "safe webhook error leaked {secret:?}: {rendered:?}" + ); + } + assert!( + rendered.starts_with("webhook transport failed ("), + "the error renders only its coarse class: {rendered:?}" + ); + } + + #[test] + fn webhook_and_graph_debug_never_render_the_endpoint() { + let secret = "https://hooks.example.invalid/debug-credential"; + let step = webhook(WebhookProvider::Discord, secret.to_string(), "done"); + let graph = AutomationGraph { + monitor: None, + parse: None, + cond: None, + timer: None, + action: None, + webhook: Some(step.clone()), + layout: None, + }; + + for rendered in [format!("{step:?}"), format!("{graph:?}")] { + assert!(rendered.contains(""), "debug did not mark the field: {rendered}"); + assert!(!rendered.contains(secret), "debug leaked the endpoint: {rendered}"); + } + } +} diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 3122cbeb..80919d3e 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -20,6 +20,7 @@ pub mod automation_commands; pub mod automation_engine; pub mod automation_store; pub mod automation_validation; +pub mod automation_webhook; pub mod canvas_endpoints; pub mod network_commands; pub mod pty_manager; @@ -1499,6 +1500,7 @@ pub fn run() { automation_commands::get_automation_runtime, automation_commands::load_automation_log, automation_commands::list_watchable_terminals, + automation_commands::preview_automation_targets, automation_commands::dry_run_automation, automation_commands::save_automation, automation_commands::add_automation_target, diff --git a/src-tauri/src/state.rs b/src-tauri/src/state.rs index 66524858..fb665d07 100644 --- a/src-tauri/src/state.rs +++ b/src-tauri/src/state.rs @@ -985,6 +985,7 @@ impl AppState { &self, process_id: &str, depth: crate::automation_engine::eval::ReadDepth, + skip_typed_line: bool, ) -> Option { use crate::automation_engine::eval::ReadDepth; let entry = self.terminal_screens.get(process_id)?; @@ -1000,7 +1001,7 @@ impl AppState { ReadDepth::Window(n) => n, ReadDepth::VisibleScreen => screen.size().0 as usize, }; - tail_text_with(screen, |sc| render_tail_lines(sc, max_lines)) + tail_text_with(screen, |sc| render_tail_lines(sc, max_lines, skip_typed_line)) } /// Escape sequences restoring the terminal's live input modes, appended to @@ -2125,6 +2126,49 @@ pub(crate) fn tail_windows( plan } +/// Which records of a tail walk make up the LOGICAL line the cursor sits on, inclusive. +/// +/// The reported problem this answers: a rule watching for `deploy` fired the instant the word +/// appeared under the user's fingers, before Enter. The screen genuinely contains that text, and +/// nothing else distinguishes an echoed keystroke from output — so the rule that opted in says +/// *the line the cursor is parked on is not output yet*. +/// +/// **The cursor's line only, never "and everything below".** The reported use is an agentic CLI +/// whose status line sits UNDER its input box, and that status line is the text such a rule is +/// watching for: dropping the rest of the screen would take the value along with the noise. +/// +/// **Both ends of a soft wrap.** One typed line can occupy several records, and it is one line to +/// the user. The backward walk is the one a long shell command needs; the forward walk is for a +/// cursor that is not on the last of those records — press Home on a wrapped command and the +/// cursor sits on its FIRST row — where dropping only from there down would leave the beginning of +/// the command behind for the pattern to match on. +/// +/// Pure, and separate from the walk, because that is what makes the span checkable as arithmetic +/// rather than through a parser: the walk's last `rows` records are the visible screen, so visible +/// row `cursor_row` is record `recs.len() - rows + cursor_row`. `None` when that lands outside the +/// walk, which is a terminal with more rows than the read's own `max_lines` — the top of its screen +/// was never in the records to begin with. +fn typed_line_span( + recs: &[(String, bool)], + rows: usize, + cursor_row: usize, +) -> Option<(usize, usize)> { + let at = recs.len().checked_add(cursor_row)?.checked_sub(rows)?; + if at >= recs.len() { + return None; + } + + let mut start = at; + while start > 0 && recs.get(start - 1).is_some_and(|(_, wrapped)| *wrapped) { + start -= 1; + } + let mut end = at; + while end + 1 < recs.len() && recs.get(end).is_some_and(|(_, wrapped)| *wrapped) { + end += 1; + } + Some((start, end)) +} + /// The last `max_lines` rows of a screen's buffer as PLAIN TEXT, soft-wrapped rows joined. /// /// **Joining wrapped rows is not optional**: a `ctx:63%` straddling column 120 otherwise never @@ -2146,10 +2190,18 @@ pub(crate) fn tail_windows( /// Bounded at `max_lines` because the walk holds the per-terminal parser mutex that `feed_screen` /// contends on, and this file's own note above `full_scrollback_snapshot` says holding it across an /// O(scrollback) render stalls output delivery for EVERY terminal. -pub fn render_tail_lines(screen: &mut vt100::Screen, max_lines: usize) -> String { +/// +/// `skip_typed_line` drops the logical line under the cursor — the rule's own opt-in, so that a +/// command still being typed is not read as output. See `typed_line_span`. +pub fn render_tail_lines(screen: &mut vt100::Screen, max_lines: usize, skip_typed_line: bool) -> String { let (rows, cols) = screen.size(); let saved = screen.scrollback(); + // Read BEFORE the walk moves the offset. The cursor belongs to the LIVE screen, and the row it + // reports is an index into the visible rows at offset 0 — asking part-way through the paging + // would be asking about whichever window happened to be showing. + let cursor_row = screen.cursor_position().0 as usize; + screen.set_scrollback(usize::MAX); let total_sb = screen.scrollback(); @@ -2168,6 +2220,22 @@ pub fn render_tail_lines(screen: &mut vt100::Screen, max_lines: usize) -> String // own scrollback view. screen.set_scrollback(saved); + // BEFORE the trailing blanks go, because the span is addressed by position in the full walk: at + // a shell prompt the typed line IS the last non-blank record, and popping first would leave the + // arithmetic pointing at whatever survived. + if skip_typed_line { + if let Some((start, end)) = typed_line_span(&recs, rows as usize, cursor_row) { + // `retain` with a counter rather than `drain(start..=end)`: constraint 2 above is that + // nothing in this walk may panic, and a range index is exactly the shape that can. + let mut i = 0usize; + recs.retain(|_| { + let keep = i < start || i > end; + i += 1; + keep + }); + } + } + while recs.last().is_some_and(|(t, _)| t.trim().is_empty()) { recs.pop(); } @@ -2273,8 +2341,9 @@ impl crate::automation_engine::host::EngineHost for AppState< &self, pc: &str, depth: crate::automation_engine::eval::ReadDepth, + skip_typed_line: bool, ) -> Option { - self.screen_tail_text(pc, depth) + self.screen_tail_text(pc, depth, skip_typed_line) } fn write(&self, pc: &str, bytes: &[u8]) -> Result<(), String> { @@ -2338,14 +2407,15 @@ impl crate::automation_engine::eval::ScreenSource for AppStat &self, process_id: &str, depth: crate::automation_engine::eval::ReadDepth, + skip_typed_line: bool, ) -> Option { - self.screen_tail_text(process_id, depth) + self.screen_tail_text(process_id, depth, skip_typed_line) } } #[cfg(test)] mod tail_read_tests { - use super::{render_tail_lines, tail_text_with, tail_windows}; + use super::{render_tail_lines, tail_text_with, tail_windows, typed_line_span}; use std::sync::Mutex; fn parser(rows: u16, cols: u16) -> vt100::Parser { @@ -2362,7 +2432,7 @@ mod tail_read_tests { let body: Vec = (1..=500).map(|i| format!("line {}", i)).collect(); p.process(body.join("\r\n").as_bytes()); - let text = render_tail_lines(p.screen_mut(), 200); + let text = render_tail_lines(p.screen_mut(), 200, false); let got: Vec<&str> = text.lines().collect(); assert_eq!(got.len(), 200, "exactly `max_lines` rows"); assert_eq!(got.first().copied(), Some("line 301")); @@ -2377,7 +2447,7 @@ mod tail_read_tests { fn a_short_buffer_returns_only_what_it_holds() { let mut p = parser(24, 80); p.process(b"alpha\r\nbeta\r\ngamma"); - let text = render_tail_lines(p.screen_mut(), 200); + let text = render_tail_lines(p.screen_mut(), 200, false); assert_eq!(text.lines().collect::>(), vec!["alpha", "beta", "gamma"]); } @@ -2386,7 +2456,7 @@ mod tail_read_tests { fn trailing_blank_rows_are_dropped() { let mut p = parser(24, 80); p.process(b"only line\r\n"); - let text = render_tail_lines(p.screen_mut(), 200); + let text = render_tail_lines(p.screen_mut(), 200, false); assert_eq!(text, "only line\n"); } @@ -2397,7 +2467,7 @@ mod tail_read_tests { let mut p = parser(10, 20); // 18 characters, then `ctx:63%` - the value straddles column 20. p.process(b"..................ctx:63%"); - let joined = render_tail_lines(p.screen_mut(), 200); + let joined = render_tail_lines(p.screen_mut(), 200, false); assert!(joined.contains("ctx:63%"), "wrapped rows must be joined: {:?}", joined); assert_eq!(joined.lines().count(), 1, "one logical line, not two physical rows"); } @@ -2408,7 +2478,7 @@ mod tail_read_tests { fn a_hard_line_break_is_not_joined() { let mut p = parser(10, 20); p.process(b"..................ct\r\nx:63%"); - let text = render_tail_lines(p.screen_mut(), 200); + let text = render_tail_lines(p.screen_mut(), 200, false); assert!(!text.contains("ctx:63%"), "a hard break is a real line end: {:?}", text); assert_eq!(text.lines().count(), 2); } @@ -2423,7 +2493,7 @@ mod tail_read_tests { p.screen_mut().set_scrollback(37); let before = p.screen().scrollback(); assert_eq!(before, 37, "premise: the view is scrolled"); - let _ = render_tail_lines(p.screen_mut(), 50); + let _ = render_tail_lines(p.screen_mut(), 50, false); assert_eq!(p.screen().scrollback(), 37, "the walk moved the user's view"); } @@ -2510,6 +2580,115 @@ mod tail_read_tests { assert!(tail_windows(0, 0, 200).is_empty()); assert!(tail_windows(500, 24, 0).is_empty()); } + + // ----------------------------------------------------------------------------------------- + // "Ignore the line being typed" — `monitor.skip_typed_line` + // ----------------------------------------------------------------------------------------- + + /// Park the cursor on a 1-based `(row, col)`, the way a shell or a TUI leaves it. + fn park(p: &mut vt100::Parser, row: u16, col: u16) { + p.process(format!("\x1b[{};{}H", row, col).as_bytes()); + } + + /// The span, as arithmetic over a walk's records — no parser, no screen. + /// + /// A table over both dimensions that can be wrong independently: WHERE the cursor is among the + /// records, and how far the soft wrap around it reaches. Varying one at a time is how an + /// implementation that ignores `rows` passes (every row of a buffer with no scrollback) or one + /// that only walks backwards passes (every cursor already at the end of its logical line). + #[test] + fn typed_line_span_is_a_table_over_position_and_wrapping() { + let plain = |n: usize| vec![(String::new(), false); n]; + // `true` means "this record soft-wraps into the next", so b/c/d are ONE logical line. + let wrapped = vec![ + (String::new(), false), // a + (String::new(), true), // b + (String::new(), true), // c + (String::new(), false), // d + (String::new(), false), // e + ]; + + // No scrollback: record index and visible row coincide. + for row in 0..5 { + assert_eq!(typed_line_span(&plain(5), 5, row), Some((row, row)), "row {}", row); + } + // With scrollback ahead of it, the visible screen is the LAST `rows` records. + assert_eq!(typed_line_span(&plain(8), 5, 2), Some((5, 5))); + assert_eq!(typed_line_span(&plain(8), 5, 0), Some((3, 3))); + + // Anywhere inside a wrapped run yields the WHOLE run, from either end of it. + assert_eq!(typed_line_span(&wrapped, 5, 1), Some((1, 3)), "from its first row"); + assert_eq!(typed_line_span(&wrapped, 5, 2), Some((1, 3)), "from its middle"); + assert_eq!(typed_line_span(&wrapped, 5, 3), Some((1, 3)), "from its last row"); + // Its neighbours are untouched by it. + assert_eq!(typed_line_span(&wrapped, 5, 0), Some((0, 0))); + assert_eq!(typed_line_span(&wrapped, 5, 4), Some((4, 4))); + + // A screen taller than the read: its top rows were never in the walk, so a cursor up there + // addresses nothing. Nothing is dropped rather than something arbitrary. + assert_eq!(typed_line_span(&plain(3), 24, 0), None); + assert_eq!(typed_line_span(&plain(3), 24, 20), None); + assert_eq!(typed_line_span(&plain(3), 24, 23), Some((2, 2)), "the bottom row still lands"); + assert_eq!(typed_line_span(&[], 24, 3), None, "an empty walk has no line to drop"); + } + + /// The reported bug: a command still being typed at a prompt fired the rule before Enter. + /// + /// Both directions in one test, because the flag is the only difference between them — an + /// implementation that ignores it passes either half alone. + #[test] + fn the_line_being_typed_is_dropped_only_when_the_rule_asks() { + let mut p = parser(6, 40); + p.process(b"build ok\r\n$ deploy now"); + + let read = render_tail_lines(p.screen_mut(), 200, false); + assert!(read.contains("deploy now"), "off, the screen is the screen: {:?}", read); + + let skipped = render_tail_lines(p.screen_mut(), 200, true); + assert!(!skipped.contains("deploy"), "the typed line survived: {:?}", skipped); + assert_eq!(skipped, "build ok\n", "and nothing above it went with it"); + } + + /// Tam's own qualifier, and the half a naive implementation fails: an agentic CLI draws its + /// status line UNDER the input box, and that status line is what the rule is watching for. + /// "The cursor's line and everything below" would take the value along with the noise. + #[test] + fn a_status_line_below_the_cursor_is_still_read() { + let mut p = parser(6, 40); + p.process(b"build ok\r\n> deploy now\r\nctx:63% . idle"); + park(&mut p, 2, 13); // back onto `> deploy now`, where a TUI leaves it + + let text = render_tail_lines(p.screen_mut(), 200, true); + // Whole-output equality, not three `contains` calls: what makes this test worth writing is + // exactly WHICH lines survived, and a `contains` oracle cannot tell "kept the status line" + // from "kept the status line and half of something else". + assert_eq!(text, "build ok\nctx:63% . idle\n"); + } + + /// A typed line long enough to wrap is still ONE line to the user, and the cursor may sit on + /// any of its rows — press Home on a long command and it sits on the FIRST. Dropping from the + /// cursor down would leave the beginning of that command behind for the pattern to match. + #[test] + fn a_soft_wrapped_typed_line_goes_whole() { + let mut p = parser(6, 20); + p.process(b"before\r\n$ deploy the whole cluster now"); + p.process(b"\x1b[5;1Hctx:63%"); + park(&mut p, 2, 3); // the first physical row of the wrapped command + + let text = render_tail_lines(p.screen_mut(), 200, true); + // Whole-output equality, and a mutation run is why. The wrap falls mid-word — the rows are + // `$ deploy the whole c` and `luster now` — so `!text.contains("cluster")` was true even + // with the continuation row still there, and a backward-only walk passed this test. + assert_eq!(text, "before\n\nctx:63%\n"); + } + + /// A cursor resting on a blank row drops a blank row — never the nearest text above it. + #[test] + fn a_cursor_on_an_empty_row_costs_nothing() { + let mut p = parser(6, 40); + p.process(b"ctx:63%\r\n"); + assert_eq!(render_tail_lines(p.screen_mut(), 200, true), "ctx:63%\n"); + } } /// §10.4c — the half of the restart guard that only a Linux CI run could otherwise check. diff --git a/src/renderer/__tests__/globalScrollbars.test.ts b/src/renderer/__tests__/globalScrollbars.test.ts new file mode 100644 index 00000000..54c29069 --- /dev/null +++ b/src/renderer/__tests__/globalScrollbars.test.ts @@ -0,0 +1,69 @@ +/** + * Scrollbars are styled ONCE, globally, and the rule must stay unscoped. + * + * Ten stylesheets had each grown their own copy, so a scrollable surface was styled if and only if + * someone remembered it. The reported failure shows why a per-component rule is not enough on its + * own: `.au-editor ::-webkit-scrollbar` covers everything inside the automation editor, and that + * editor's dropdown portals to `body` precisely so a clipping ancestor cannot cut it off — which + * puts it outside the selector, and drew a bare Win32 scrollbar in the middle of a dark modal. + * + * jsdom has no cascade and no scrollbars at all, so the stylesheet is the only place this can be + * asked. The assertion that carries the weight is the SCOPE: a rule written as + * `.something ::-webkit-scrollbar` in this file would pass a naive "is it styled" check while + * leaving every portalled surface bare again. + */ +import fs from 'fs'; +import path from 'path'; + +const CSS = fs.readFileSync( + path.join(__dirname, '..', 'styles', 'index.css'), + 'utf8', +); + +/** + * Comments stripped FIRST. A block comment contains no braces, so without this every prose + * paragraph above a rule is swallowed into that rule's "selector" — which made the scope + * assertion below fail on the very file it was written to pass. + */ +const RULES = CSS.replace(/\/\*[\s\S]*?\*\//g, ''); + +/** Every selector in the file that mentions a scrollbar pseudo-element. */ +const scrollbarSelectors = [...RULES.matchAll(/([^{}]*::-webkit-scrollbar[^{}]*)\{/g)] + .map((m) => m[1].trim()); + +describe('global scrollbar styling', () => { + it('styles the track, the thumb, its hover and the corner', () => { + for (const part of ['', '-track', '-thumb', '-corner']) { + expect(scrollbarSelectors).toContain(`::-webkit-scrollbar${part}`); + } + expect(scrollbarSelectors).toContain('::-webkit-scrollbar-thumb:hover'); + }); + + it('leaves every rule unscoped, so a portalled surface is covered too', () => { + // A selector with anything before the pseudo-element is a descendant rule, and a descendant + // rule cannot follow a `createPortal` out to `body`. + for (const selector of scrollbarSelectors) { + expect(selector.startsWith('::-webkit-scrollbar')).toBe(true); + } + }); + + it('states the standard properties as well as the WebKit ones', () => { + // WebKit ignores `scrollbar-width`/`scrollbar-color` and uses the pseudo-elements; other + // engines do the reverse. Either alone leaves one of them on the default chrome. + expect(CSS).toMatch(/scrollbar-width:\s*thin/); + expect(CSS).toMatch(/scrollbar-color:\s*#[0-9a-f]{6}\s+transparent/i); + }); + + /** + * The thumb's inset comes from a transparent border clipped to the padding box, NOT from a + * background colour matching one panel. The editor's own copy hard-coded `#17181c` — its + * modal's ground — and lifting that verbatim would have drawn a dark-grey halo around the + * thumb on every surface with a different background. + */ + it('insets the thumb without pinning one panel background app-wide', () => { + const thumb = RULES.slice(RULES.indexOf('::-webkit-scrollbar-thumb {')); + const body = thumb.slice(thumb.indexOf('{') + 1, thumb.indexOf('}')); + expect(body).toMatch(/border:\s*2px solid transparent/); + expect(body).toMatch(/background-clip:\s*padding-box/); + }); +}); diff --git a/src/renderer/api/tauri-bridge.ts b/src/renderer/api/tauri-bridge.ts index 7b8382be..891b452c 100644 --- a/src/renderer/api/tauri-bridge.ts +++ b/src/renderer/api/tauri-bridge.ts @@ -3,7 +3,7 @@ import { listen } from '@tauri-apps/api/event'; import type { UnlistenFn } from '@tauri-apps/api/event'; import { getCurrentWindow } from '@tauri-apps/api/window'; import { open as openFileDialog, save as saveFileDialog } from '@tauri-apps/plugin-dialog'; -import type { TerminalSnapshot, ActiveProcess, PeerInfo, PeerRequestInfo, PairingCode, FabricStatus, GrantLevel, AutomationRule, AutomationLogEntry, AutomationSaveResult, WatchableTerminal, DryRunReport } from '../types/electron'; +import type { TerminalSnapshot, ActiveProcess, PeerInfo, PeerRequestInfo, PairingCode, FabricStatus, GrantLevel, AutomationCriterion, AutomationRule, AutomationLogEntry, AutomationSaveResult, WatchableTerminal, AutomationTargetPreview, DryRunReport } from '../types/electron'; import type { AutomationStatePayload } from '../services/automationEvents'; import { shouldHandleForWindow } from './windowRouting'; import { emitPtyInput } from '../utils/ptyInputSignal'; @@ -221,7 +221,8 @@ interface ElectronAPI { listAutomations: () => Promise; getAutomationRuntime: () => Promise; loadAutomationLog: (ruleId: string | null, newestFirst: boolean, limit: number) => Promise; - listWatchableTerminals: (ruleId: string | null, includeIds: string[] | null) => Promise; + listWatchableTerminals: (ruleId: string | null, includeIds: string[] | null, criteria: AutomationCriterion[]) => Promise; + previewAutomationTargets: (rule: AutomationRule, terminals: WatchableTerminal[]) => Promise; dryRunAutomation: (rule: AutomationRule, terminalId: string) => Promise; saveAutomation: (rule: AutomationRule, origin: string) => Promise; deleteAutomation: (id: string, origin: string) => Promise; @@ -893,7 +894,7 @@ const tauriBridge: ElectronAPI = { // --- Terminal Automations (Plan 028) --- // // Thin `invoke` wrappers over `automation_commands.rs`, one per command. The first - // eleven follow that file's declaration order; the three id-only writers that replaced a + // twelve follow that file's declaration order; the three id-only writers that replaced a // whole-rule `saveAutomation` are appended last here as the newest of them, wherever // they sit in the Rust module. Every argument name // here is the camelCase form Tauri derives from the Rust parameter — `rule_id` on the @@ -904,8 +905,10 @@ const tauriBridge: ElectronAPI = { getAutomationRuntime: async () => invoke('get_automation_runtime'), loadAutomationLog: async (ruleId, newestFirst, limit) => invoke('load_automation_log', { ruleId, newestFirst, limit }), - listWatchableTerminals: async (ruleId, includeIds) => - invoke('list_watchable_terminals', { ruleId, includeIds }), + listWatchableTerminals: async (ruleId, includeIds, criteria) => + invoke('list_watchable_terminals', { ruleId, includeIds, criteria }), + previewAutomationTargets: async (rule, terminals) => + invoke('preview_automation_targets', { rule, terminals }), dryRunAutomation: async (rule, terminalId) => invoke('dry_run_automation', { rule, terminalId }), saveAutomation: async (rule, origin) => diff --git a/src/renderer/components/Automation/AuActivityPane.tsx b/src/renderer/components/Automation/AuActivityPane.tsx index b5866ec4..9403a742 100644 --- a/src/renderer/components/Automation/AuActivityPane.tsx +++ b/src/renderer/components/Automation/AuActivityPane.tsx @@ -18,6 +18,7 @@ import React from 'react'; import type { AutomationLogEntry } from '../../types/electron'; import { LOG_KIND_CLASS, LOG_KIND_LABEL, clockTime } from '../Settings/Automations/activityLog'; +import { redactWebhookLogEntry } from './webhookRedaction'; export interface AuActivityPaneProps { entries: AutomationLogEntry[]; @@ -32,7 +33,11 @@ export const AuActivityPane: React.FC = ({ error, saved, onOpenFullLog, -}) => ( +}) => { + // Keep this drawer independent of the Settings log's fetch path while applying the same + // last-resort redaction to every field it renders. + const redactedEntries = entries.map(redactWebhookLogEntry); + return (
{error !== null && (
@@ -43,7 +48,7 @@ export const AuActivityPane: React.FC = ({
)} - {error === null && entries.length === 0 && ( + {error === null && redactedEntries.length === 0 && (
No activity yet.
@@ -53,10 +58,10 @@ export const AuActivityPane: React.FC = ({
)} - {error === null && entries.length > 0 && ( + {error === null && redactedEntries.length > 0 && ( <>
- {entries.map((entry) => ( + {redactedEntries.map((entry) => (
{clockTime(entry.at)} @@ -78,4 +83,5 @@ export const AuActivityPane: React.FC = ({ )}
-); + ); +}; diff --git a/src/renderer/components/Automation/AuCanvas.tsx b/src/renderer/components/Automation/AuCanvas.tsx index 325bf2d2..36d21902 100644 --- a/src/renderer/components/Automation/AuCanvas.tsx +++ b/src/renderer/components/Automation/AuCanvas.tsx @@ -21,7 +21,9 @@ import type { AutomationDraft, NodePos } from './automationDraft'; import { AU_NODE_H, AU_NODE_W, portSides } from './automationDraft'; import type { NodeFace, NodeState } from './automationDerive'; import type { PortRef, StepKind, Wire } from './automationSteps'; +import { removalGroup } from './automationSteps'; import { AuNode } from './AuNode'; +import { AuNodeMenu } from './AuNodeMenu'; import { AuWires } from './AuWires'; import { useAuNodeDrag } from './useAuNodeDrag'; import { useAuWireDrag } from './useAuWireDrag'; @@ -38,6 +40,12 @@ export interface AuCanvasProps { onMove: (step: StepKind, pos: NodePos) => void; onConnect: (wire: Wire) => void; onDisconnect: (wire: Wire) => void; + /** + * Take a step off the canvas. **The step the gesture was aimed at**, not the set that will go: + * the editor asks `removalGroup` for that, so the reducer, the menu's label and the toast are + * all reading one answer rather than three. + */ + onRemove: (step: StepKind) => void; onRefuse: (reason: string) => void; /** The palette drag needs screen → world too, and only this component knows the transform. */ onViewportReady: (toWorld: (x: number, y: number) => NodePos | null) => void; @@ -53,6 +61,7 @@ export const AuCanvas: React.FC = ({ onMove, onConnect, onDisconnect, + onRemove, onRefuse, onViewportReady, children, @@ -63,6 +72,8 @@ export const AuCanvas: React.FC = ({ const dpr = typeof window === 'undefined' ? 1 : (window.devicePixelRatio || 1); const [vp, setVp] = useState({ x: 0, y: 0, z: 1 }); const [spacePan, setSpacePan] = useState(false); + // Where the right-click menu is, and which card it was aimed at. `null` is closed. + const [menu, setMenu] = useState<{ step: StepKind; x: number; y: number } | null>(null); const panning = useRef<{ x: number; y: number } | null>(null); const toWorldOrNull = useCallback((clientX: number, clientY: number): NodePos | null => { @@ -289,6 +300,17 @@ export const AuCanvas: React.FC = ({ dropPorts={dropPorts[step]} sides={sides} onSelect={() => onSelect(step)} + onDelete={() => onRemove(step)} + onContextMenu={(e) => { + e.preventDefault(); + // Or the canvas beneath opens a second menu on the same press. + e.stopPropagation(); + // A right-click SELECTS as well, so the inspector is showing the card + // the menu is about — a menu offering to delete one step over a panel + // describing another is the same drift the label rule above avoids. + onSelect(step); + setMenu({ step, x: e.clientX, y: e.clientY }); + }} // Space-pan wins over a node drag. React's bubble handler on the node runs // BEFORE the canvas's own, so without this a space+drag that happened to // start on a card moved the card AND the viewport, by the same delta, in @@ -327,6 +349,16 @@ export const AuCanvas: React.FC = ({
+ {menu && ( + setMenu(null)} + onDelete={() => onRemove(menu.step)} + /> + )} + {children}
); diff --git a/src/renderer/components/Automation/AuInfo.tsx b/src/renderer/components/Automation/AuInfo.tsx new file mode 100644 index 00000000..66aa7615 --- /dev/null +++ b/src/renderer/components/Automation/AuInfo.tsx @@ -0,0 +1,146 @@ +/** + * An **ⓘ** beside a field label, and the styled panel it opens. + * + * **Why not `title`.** A native tooltip is the browser's: one size, one colour, no markup, a delay + * before it appears and a timeout that takes it away mid-sentence. What belongs behind this icon is + * a few worked examples — several lines, a `` run per example, a heading per case — which is + * not something a `title` string can hold at all. It also has to be reachable by keyboard and by + * touch, and a hover tooltip is neither. + * + * **Click, not hover**, for the same reason: the panel is content to read, and content that + * vanishes when the pointer travels toward it cannot be read. + * + * **Portalled and `fixed`, like `AuSelect` and `AuTerminalHoverCard`.** The inspector column is + * `overflow-y: auto` and the editor's `.au-modal` is `overflow: hidden`, so a panel rendered in + * flow is clipped twice over. Every placement decision therefore has to be made against the + * trigger's `getBoundingClientRect()` — see `infoPopPosition`, which is a pure function for the + * same reason the rest of this folder's geometry is: jsdom lays nothing out, and a placement that + * can only be checked by eye is a placement nobody checks. + */ +import React, { useCallback, useEffect, useLayoutEffect, useRef, useState } from 'react'; +import { createPortal } from 'react-dom'; + +/** The panel's box. Needed as NUMBERS, not only as CSS — `infoPopPosition` reasons about them. */ +export const AU_INFO_W = 320; +/** + * The tallest it may get, and the height the clamp assumes. Measuring the rendered element instead + * would be exact and would cost a second render pass; a `max-height` the CSS also enforces cannot + * be wrong in the direction that matters — the panel can be shorter and still fit, never taller. + */ +export const AU_INFO_MAX_H = 300; +/** Air between the panel and its icon. */ +const GAP = 6; +/** Air between the panel and the window edge, on every side. */ +const MARGIN = 8; + +/** + * Where the panel goes, given the icon it belongs to and the window it has to stay inside. + * + * **Below unless it does not fit, then above** — the icon sits in a label at the top of a field, so + * below is where there is normally room and where the eye already is. Horizontally the panel hangs + * from the icon's LEFT edge, because these labels are left-aligned and a panel that grew leftwards + * from a left-hand icon would sit over the margin. + * + * Both clamps end with the low edge, deliberately: on a window smaller than the panel the top-left + * is the corner that wins, so it is never pushed off the side or the top you read from. + */ +export function infoPopPosition( + anchor: { top: number; bottom: number; left: number }, + view: { width: number; height: number }, +): { left: number; top: number } { + let left = anchor.left; + if (left + AU_INFO_W > view.width - MARGIN) left = view.width - MARGIN - AU_INFO_W; + if (left < MARGIN) left = MARGIN; + + const below = anchor.bottom + GAP; + // Above only when below genuinely cannot hold it, so the panel does not jump sides as the + // inspector scrolls a few pixels. + const top = below + AU_INFO_MAX_H <= view.height - MARGIN + ? below + : Math.max(MARGIN, anchor.top - GAP - AU_INFO_MAX_H); + return { left, top }; +} + +export interface AuInfoProps { + /** Names the button AND the panel: a screen reader meets the panel with no icon to look at. */ + label: string; + children: React.ReactNode; +} + +export const AuInfo: React.FC = ({ label, children }) => { + const [open, setOpen] = useState(false); + const [at, setAt] = useState<{ left: number; top: number } | null>(null); + const triggerRef = useRef(null); + const popRef = useRef(null); + + const place = useCallback(() => { + const el = triggerRef.current; + if (!el) return; + const r = el.getBoundingClientRect(); + setAt(infoPopPosition(r, { width: window.innerWidth, height: window.innerHeight })); + }, []); + + useLayoutEffect(() => { + if (!open) return; + place(); + // Capture, so a scroll of the inspector column repositions too and not only one of the + // window itself — the same reason `AuSelect` gives. + window.addEventListener('resize', place); + window.addEventListener('scroll', place, true); + return () => { + window.removeEventListener('resize', place); + window.removeEventListener('scroll', place, true); + }; + }, [open, place]); + + useEffect(() => { + if (!open) return; + const onDown = (e: MouseEvent) => { + const target = e.target as Node; + // The panel itself is exempt: it holds text people select and links they may click. + if (popRef.current?.contains(target) || triggerRef.current?.contains(target)) return; + setOpen(false); + }; + const onKey = (e: KeyboardEvent) => { + if (e.key !== 'Escape') return; + // Stopped here so the editor's own Escape does not ALSO close the whole dialog behind + // it: dismissing a panel and discarding an unsaved rule are not the same gesture. + e.stopPropagation(); + setOpen(false); + triggerRef.current?.focus(); + }; + document.addEventListener('mousedown', onDown); + document.addEventListener('keydown', onKey, true); + return () => { + document.removeEventListener('mousedown', onDown); + document.removeEventListener('keydown', onKey, true); + }; + }, [open]); + + return ( + <> + + {open && at && createPortal( +
+ {children} +
, + document.body, + )} + + ); +}; diff --git a/src/renderer/components/Automation/AuInspector.tsx b/src/renderer/components/Automation/AuInspector.tsx index 9fb2ffff..3a89b90e 100644 --- a/src/renderer/components/Automation/AuInspector.tsx +++ b/src/renderer/components/Automation/AuInspector.tsx @@ -21,6 +21,8 @@ import { ParsePanel } from './panels/ParsePanel'; import { CondPanel } from './panels/CondPanel'; import { TimerPanel } from './panels/TimerPanel'; import { ActionPanel } from './panels/ActionPanel'; +import { WebhookPanel } from './panels/WebhookPanel'; +import { redactWebhookText } from './webhookRedaction'; export interface AuInspectorProps { draft: AutomationDraft; @@ -61,6 +63,7 @@ const FIELD_STEPS: Record = { cond: 'cond', timer: 'timer', action: 'action', + webhook: 'webhook', }; export const AuInspector: React.FC = (props) => { @@ -133,9 +136,12 @@ export const AuInspector: React.FC = (props) => { {step === 'timer' && ( )} - {step === 'action' && ( + {step === 'action' && draft.rule.graph.action && ( )} + {step === 'webhook' && draft.rule.graph.webhook && ( + + )} ); @@ -158,9 +164,9 @@ const ProblemList: React.FC<{ problems: Problem[]; onFocusStep: (step: StepKind)
    {problems.map((p) => ( -
  • +
  • ))} diff --git a/src/renderer/components/Automation/AuInspectorDock.tsx b/src/renderer/components/Automation/AuInspectorDock.tsx new file mode 100644 index 00000000..32621927 --- /dev/null +++ b/src/renderer/components/Automation/AuInspectorDock.tsx @@ -0,0 +1,129 @@ +/** + * The right-hand column's chrome: **how wide it is, and whether it is there at all.** + * + * The inspector itself stays exactly what it was — a pure projection of the draft — and this wraps + * it rather than growing it two more responsibilities. That also keeps `AuInspector`'s signature + * unchanged, so the six test call sites that mount it directly are unaffected. + * + * **The dock is the grid child now, not the `