Skip to content

Commit 9c3f971

Browse files
committed
Fix trigger overlap validation
1 parent 1e48dca commit 9c3f971

3 files changed

Lines changed: 99 additions & 54 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ All notable changes to `devloop` will be recorded in this file.
2222
trigger target is also reachable through `run_workflow`, and
2323
triggered workflows are documented as single-run deduplicated within
2424
one execution.
25+
- Trigger-overlap validation now walks the full execution tree, so
26+
nested trigger graphs cannot schedule the same workflow once as a
27+
trigger target and again through an inline `run_workflow` path.
2528
- Platform-specific release workflows no longer duplicate GitHub release notes when both assets are published to the same tag.
2629

2730
## [0.6.2] - 2026-03-26

examples/blog/devloop.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ triggers = ["browser_reload"]
9090

9191
[workflow.content]
9292
steps = [
93+
{ action = "run_hook", hook = "build_css" },
9394
{ action = "run_hook", hook = "current_browser_path" },
9495
{ action = "restart_process", process = "tunnel" },
9596
{ action = "run_workflow", workflow = "publish_post_url" },

src/config.rs

Lines changed: 95 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::collections::{BTreeMap, HashSet};
1+
use std::collections::{BTreeMap, BTreeSet, HashSet};
22
use std::net::SocketAddr;
33
use std::path::{Path, PathBuf};
44
use std::time::Duration;
@@ -105,7 +105,7 @@ impl Config {
105105
}
106106
for (name, workflow) in &self.workflow {
107107
workflow
108-
.validate(self)
108+
.validate(self, name)
109109
.with_context(|| format!("invalid workflow '{name}'"))?;
110110
}
111111
for workflow in &self.startup_workflows {
@@ -530,15 +530,19 @@ pub struct WorkflowSpec {
530530
}
531531

532532
impl WorkflowSpec {
533-
fn validate(&self, config: &Config) -> Result<()> {
534-
self.validate_inner(config, &mut Vec::new())
533+
fn validate(&self, config: &Config, workflow_name: &str) -> Result<()> {
534+
self.validate_inner(config, workflow_name, &mut Vec::new())
535535
}
536536

537-
fn validate_inner(&self, config: &Config, stack: &mut Vec<String>) -> Result<()> {
537+
fn validate_inner(
538+
&self,
539+
config: &Config,
540+
workflow_name: &str,
541+
stack: &mut Vec<String>,
542+
) -> Result<()> {
538543
if self.steps.is_empty() {
539544
return Err(anyhow!("workflow must contain at least one step"));
540545
}
541-
let mut inline_workflows = HashSet::new();
542546
for step in &self.steps {
543547
match step {
544548
WorkflowStep::StartProcess { process }
@@ -555,7 +559,6 @@ impl WorkflowSpec {
555559
}
556560
}
557561
WorkflowStep::RunWorkflow { workflow } => {
558-
inline_workflows.insert(workflow.clone());
559562
validate_nested_workflow(config, stack, workflow)?;
560563
}
561564
WorkflowStep::SleepMs { .. }
@@ -565,53 +568,38 @@ impl WorkflowSpec {
565568
}
566569
}
567570
for workflow in &self.triggers {
568-
if inline_workflows.contains(workflow) {
569-
return Err(anyhow!(
570-
"workflow cannot both run_workflow and trigger '{workflow}'"
571-
));
572-
}
573571
validate_nested_workflow(config, stack, workflow)?;
574572
}
575-
if !self.triggers.is_empty() && (!inline_workflows.is_empty() || self.triggers.len() > 1) {
576-
validate_trigger_inline_overlap(config, &inline_workflows, &self.triggers)?;
573+
if !self.triggers.is_empty() {
574+
validate_execution_tree_overlap(config, workflow_name)?;
577575
}
578576
Ok(())
579577
}
580578
}
581579

582-
fn validate_trigger_inline_overlap(
583-
config: &Config,
584-
inline_workflows: &HashSet<String>,
585-
triggers: &[String],
586-
) -> Result<()> {
587-
let mut sorted_inline_workflows = inline_workflows.iter().collect::<Vec<_>>();
588-
sorted_inline_workflows.sort();
589-
for trigger in triggers {
590-
for inline_workflow in &sorted_inline_workflows {
591-
if workflow_reaches_via_inline_execution(config, inline_workflow, trigger) {
592-
return Err(anyhow!(
593-
"workflow trigger '{trigger}' is also reachable via run_workflow from '{inline_workflow}'"
594-
));
595-
}
596-
}
597-
for sibling_trigger in triggers {
598-
if sibling_trigger == trigger {
599-
continue;
600-
}
601-
if workflow_reaches_via_inline_execution(config, sibling_trigger, trigger) {
602-
return Err(anyhow!(
603-
"workflow trigger '{trigger}' is also reachable via run_workflow from sibling trigger '{sibling_trigger}'"
604-
));
605-
}
580+
fn validate_execution_tree_overlap(config: &Config, workflow_name: &str) -> Result<()> {
581+
let (trigger_targets, inline_reachable) = execution_tree_overlap_sets(config, workflow_name);
582+
583+
for workflow in trigger_targets {
584+
if inline_reachable.contains(&workflow) {
585+
return Err(anyhow!(
586+
"workflow '{workflow}' is reachable both as a trigger target and via run_workflow in the same execution tree"
587+
));
606588
}
607589
}
590+
608591
Ok(())
609592
}
610593

611-
fn workflow_reaches_via_inline_execution(config: &Config, start: &str, target: &str) -> bool {
612-
let mut stack = vec![(start.to_string(), false)];
594+
fn execution_tree_overlap_sets(
595+
config: &Config,
596+
workflow_name: &str,
597+
) -> (BTreeSet<String>, BTreeSet<String>) {
598+
let mut stack = vec![(workflow_name.to_string(), false)];
613599
let mut seen_plain = HashSet::new();
614600
let mut seen_inline = HashSet::new();
601+
let mut trigger_targets = BTreeSet::new();
602+
let mut inline_reachable = BTreeSet::new();
615603

616604
while let Some((workflow_name, used_inline)) = stack.pop() {
617605
let seen_set = if used_inline {
@@ -622,8 +610,8 @@ fn workflow_reaches_via_inline_execution(config: &Config, start: &str, target: &
622610
if !seen_set.insert(workflow_name.clone()) {
623611
continue;
624612
}
625-
if workflow_name == target && used_inline {
626-
return true;
613+
if used_inline {
614+
inline_reachable.insert(workflow_name.clone());
627615
}
628616

629617
let Some(workflow) = config.workflow.get(&workflow_name) else {
@@ -636,11 +624,12 @@ fn workflow_reaches_via_inline_execution(config: &Config, start: &str, target: &
636624
}
637625
}
638626
for trigger in &workflow.triggers {
627+
trigger_targets.insert(trigger.clone());
639628
stack.push((trigger.clone(), used_inline));
640629
}
641630
}
642631

643-
false
632+
(trigger_targets, inline_reachable)
644633
}
645634

646635
fn validate_nested_workflow(
@@ -661,7 +650,7 @@ fn validate_nested_workflow(
661650
.get(workflow)
662651
.ok_or_else(|| anyhow!("workflow references missing workflow '{workflow}'"))?;
663652
stack.push(workflow.to_string());
664-
nested.validate_inner(config, stack)?;
653+
nested.validate_inner(config, workflow, stack)?;
665654
stack.pop();
666655
Ok(())
667656
}
@@ -804,7 +793,7 @@ mod tests {
804793
);
805794

806795
let error = config.workflow["outer"]
807-
.validate(&config)
796+
.validate(&config, "outer")
808797
.expect_err("recursive workflow should fail");
809798
assert!(error.to_string().contains("workflow recursion detected"));
810799
}
@@ -834,7 +823,7 @@ mod tests {
834823
);
835824

836825
let error = config.workflow["outer"]
837-
.validate(&config)
826+
.validate(&config, "outer")
838827
.expect_err("recursive trigger workflow should fail");
839828
assert!(error.to_string().contains("workflow recursion detected"));
840829
}
@@ -853,7 +842,7 @@ mod tests {
853842
);
854843

855844
let error = config.workflow["outer"]
856-
.validate(&config)
845+
.validate(&config, "outer")
857846
.expect_err("missing nested workflow should fail");
858847
assert!(error.to_string().contains("missing workflow 'missing'"));
859848
}
@@ -873,7 +862,7 @@ mod tests {
873862
);
874863

875864
let error = config.workflow["outer"]
876-
.validate(&config)
865+
.validate(&config, "outer")
877866
.expect_err("missing trigger workflow should fail");
878867
assert!(error.to_string().contains("missing workflow 'missing'"));
879868
}
@@ -899,12 +888,12 @@ mod tests {
899888
);
900889

901890
let error = config.workflow["css"]
902-
.validate(&config)
891+
.validate(&config, "css")
903892
.expect_err("overlapping trigger and inline workflow should fail");
904893
assert!(
905894
error
906895
.to_string()
907-
.contains("workflow cannot both run_workflow and trigger 'browser_reload'")
896+
.contains("reachable both as a trigger target and via run_workflow")
908897
);
909898
}
910899

@@ -939,11 +928,63 @@ mod tests {
939928
);
940929

941930
let error = config.workflow["a"]
942-
.validate(&config)
931+
.validate(&config, "a")
943932
.expect_err("sibling trigger overlap should fail");
944-
assert!(error.to_string().contains(
945-
"workflow trigger 'c' is also reachable via run_workflow from sibling trigger 'b'"
946-
));
933+
assert!(
934+
error.to_string().contains(
935+
"workflow 'c' is reachable both as a trigger target and via run_workflow"
936+
)
937+
);
938+
}
939+
940+
#[test]
941+
fn validate_rejects_nested_trigger_reachable_via_inline_path() {
942+
let mut config = base_config();
943+
config.workflow.insert(
944+
"d".into(),
945+
WorkflowSpec {
946+
steps: vec![WorkflowStep::NotifyReload],
947+
triggers: vec![],
948+
},
949+
);
950+
config.workflow.insert(
951+
"b".into(),
952+
WorkflowSpec {
953+
steps: vec![WorkflowStep::Log {
954+
message: "b".into(),
955+
style: LogStyle::Plain,
956+
}],
957+
triggers: vec!["d".into()],
958+
},
959+
);
960+
config.workflow.insert(
961+
"c".into(),
962+
WorkflowSpec {
963+
steps: vec![WorkflowStep::RunWorkflow {
964+
workflow: "d".into(),
965+
}],
966+
triggers: vec![],
967+
},
968+
);
969+
config.workflow.insert(
970+
"a".into(),
971+
WorkflowSpec {
972+
steps: vec![WorkflowStep::Log {
973+
message: "a".into(),
974+
style: LogStyle::Plain,
975+
}],
976+
triggers: vec!["b".into(), "c".into()],
977+
},
978+
);
979+
980+
let error = config.workflow["a"]
981+
.validate(&config, "a")
982+
.expect_err("nested trigger overlap should fail");
983+
assert!(
984+
error.to_string().contains(
985+
"workflow 'd' is reachable both as a trigger target and via run_workflow"
986+
)
987+
);
947988
}
948989

949990
#[test]

0 commit comments

Comments
 (0)