fix: Fix cycle in affected tracker.

CHANGELOG.md

@@ -6,6 +6,7 @@
 
 - Fixed an issue where dependencies/dependents of an affected task would be skipped in the action
   graph if they were also not affected.
+- Fixed a potential cycle (stack overflow) that may occur in the affected tracker.
 
 #### ⚙️ Internal
 

crates/affected/src/affected_tracker.rs

@@ -151,6 +151,10 @@ impl<'app> AffectedTracker<'app> {
         }
     }
 
+    pub fn is_project_marked(&self, project: &Project) -> bool {
+        self.projects.contains_key(&project.id)
+    }
+
     pub fn mark_project_affected(
         &mut self,
         project: &Project,
@@ -170,13 +174,24 @@ impl<'app> AffectedTracker<'app> {
             .or_default()
             .insert(affected);
 
-        self.track_project_dependencies(project, 0)?;
-        self.track_project_dependents(project, 0)?;
+        self.track_project_dependencies(project, 0, &mut FxHashSet::default())?;
+        self.track_project_dependents(project, 0, &mut FxHashSet::default())?;
 
         Ok(())
     }
 
-    fn track_project_dependencies(&mut self, project: &Project, depth: u16) -> miette::Result<()> {
+    fn track_project_dependencies(
+        &mut self,
+        project: &Project,
+        depth: u16,
+        cycle: &mut FxHashSet<Id>,
+    ) -> miette::Result<()> {
+        if cycle.contains(&project.id) {
+            return Ok(());
+        }
+
+        cycle.insert(project.id.clone());
+
         if self.project_upstream == UpstreamScope::None {
             trace!(
                 project_id = project.id.as_str(),
@@ -212,13 +227,24 @@ impl<'app> AffectedTracker<'app> {
 
             let dep_project = self.workspace_graph.get_project(&dep_config.id)?;
 
-            self.track_project_dependencies(&dep_project, depth + 1)?;
+            self.track_project_dependencies(&dep_project, depth + 1, cycle)?;
         }
 
         Ok(())
     }
 
-    fn track_project_dependents(&mut self, project: &Project, depth: u16) -> miette::Result<()> {
+    fn track_project_dependents(
+        &mut self,
+        project: &Project,
+        depth: u16,
+        cycle: &mut FxHashSet<Id>,
+    ) -> miette::Result<()> {
+        if cycle.contains(&project.id) {
+            return Ok(());
+        }
+
+        cycle.insert(project.id.clone());
+
         if self.project_downstream == DownstreamScope::None {
             trace!(
                 project_id = project.id.as_str(),
@@ -254,7 +280,7 @@ impl<'app> AffectedTracker<'app> {
 
             let dep_project = self.workspace_graph.get_project(&dep_id)?;
 
-            self.track_project_dependents(&dep_project, depth + 1)?;
+            self.track_project_dependents(&dep_project, depth + 1, cycle)?;
         }
 
         Ok(())
@@ -339,8 +365,8 @@ impl<'app> AffectedTracker<'app> {
             .or_default()
             .insert(affected);
 
-        self.track_task_dependencies(task, 0)?;
-        self.track_task_dependents(task, 0)?;
+        self.track_task_dependencies(task, 0, &mut FxHashSet::default())?;
+        self.track_task_dependents(task, 0, &mut FxHashSet::default())?;
 
         if let Some(project_id) = task.target.get_project_id() {
             self.projects
@@ -352,7 +378,18 @@ impl<'app> AffectedTracker<'app> {
         Ok(())
     }
 
-    fn track_task_dependencies(&mut self, task: &Task, depth: u16) -> miette::Result<()> {
+    fn track_task_dependencies(
+        &mut self,
+        task: &Task,
+        depth: u16,
+        cycle: &mut FxHashSet<Target>,
+    ) -> miette::Result<()> {
+        if cycle.contains(&task.target) {
+            return Ok(());
+        }
+
+        cycle.insert(task.target.clone());
+
         if self.task_upstream == UpstreamScope::None {
             trace!(
                 task_target = task.target.as_str(),
@@ -388,13 +425,24 @@ impl<'app> AffectedTracker<'app> {
 
             let dep_task = self.workspace_graph.get_task(&dep_config.target)?;
 
-            self.track_task_dependencies(&dep_task, depth + 1)?;
+            self.track_task_dependencies(&dep_task, depth + 1, cycle)?;
         }
 
         Ok(())
     }
 
-    fn track_task_dependents(&mut self, task: &Task, depth: u16) -> miette::Result<()> {
+    fn track_task_dependents(
+        &mut self,
+        task: &Task,
+        depth: u16,
+        cycle: &mut FxHashSet<Target>,
+    ) -> miette::Result<()> {
+        if cycle.contains(&task.target) {
+            return Ok(());
+        }
+
+        cycle.insert(task.target.clone());
+
         if self.task_downstream == DownstreamScope::None {
             trace!(
                 task_target = task.target.as_str(),
@@ -430,7 +478,7 @@ impl<'app> AffectedTracker<'app> {
 
             let dep_task = self.workspace_graph.get_task(&dep_target)?;
 
-            self.track_task_dependents(&dep_task, depth + 1)?;
+            self.track_task_dependents(&dep_task, depth + 1, cycle)?;
         }
 
         Ok(())

crates/affected/tests/__fixtures__/projects/cycle-a/moon.yml

@@ -0,0 +1 @@
+dependsOn: ['cycle-b']

crates/affected/tests/__fixtures__/projects/cycle-b/moon.yml

@@ -0,0 +1 @@
+dependsOn: ['cycle-c']

crates/affected/tests/__fixtures__/projects/cycle-c/moon.yml

@@ -0,0 +1 @@
+dependsOn: ['cycle-a']

crates/affected/tests/__fixtures__/tasks/cycle/moon.yml

@@ -0,0 +1,10 @@
+tasks:
+  a:
+    deps: ['b']
+    inputs: ['a.txt']
+  b:
+    deps: ['c']
+    inputs: ['b.txt']
+  c:
+    deps: ['a']
+    inputs: ['c.txt']

crates/affected/tests/affected_tracker_test.rs

@@ -198,6 +198,33 @@ mod affected_projects {
                 ])
             );
         }
+
+        #[tokio::test]
+        async fn deep_cycle() {
+            use moon_test_utils2::pretty_assertions::assert_eq;
+
+            let workspace_graph = generate_workspace_graph("projects").await;
+            let touched_files = FxHashSet::from_iter(["cycle-a/file.txt".into()]);
+
+            let mut tracker = AffectedTracker::new(&workspace_graph, &touched_files);
+            tracker.with_project_scopes(UpstreamScope::Deep, DownstreamScope::None);
+            tracker.track_projects().unwrap();
+            let affected = tracker.build();
+
+            assert_eq!(
+                affected.projects,
+                FxHashMap::from_iter([
+                    (Id::raw("cycle-a"), {
+                        let mut state = create_state_from_file("cycle-a/file.txt");
+                        state.downstream.insert(Id::raw("cycle-c"));
+                        state
+                    }),
+                    (Id::raw("cycle-b"), create_state_from_dependent("cycle-a")),
+                    (Id::raw("cycle-c"), create_state_from_dependent("cycle-b")),
+                    (Id::raw("root"), create_state_from_file("cycle-a/file.txt")),
+                ])
+            );
+        }
     }
 
     mod project_downstream {
@@ -301,6 +328,30 @@ mod affected_projects {
                 ])
             );
         }
+
+        #[tokio::test]
+        async fn deep_cycle() {
+            let workspace_graph = generate_workspace_graph("projects").await;
+            let touched_files = FxHashSet::from_iter(["cycle-c/file.txt".into()]);
+
+            let mut tracker = AffectedTracker::new(&workspace_graph, &touched_files);
+            tracker.with_project_scopes(UpstreamScope::None, DownstreamScope::Deep);
+            tracker.track_projects().unwrap();
+            let affected = tracker.build();
+
+            assert_eq!(
+                affected.projects,
+                FxHashMap::from_iter([
+                    (Id::raw("cycle-a"), create_state_from_dependency("cycle-b")),
+                    (Id::raw("cycle-b"), create_state_from_dependency("cycle-c")),
+                    (
+                        Id::raw("cycle-c"),
+                        create_state_from_file("cycle-c/file.txt")
+                    ),
+                    (Id::raw("root"), create_state_from_file("cycle-c/file.txt")),
+                ])
+            );
+        }
     }
 }
 
@@ -644,11 +695,44 @@ mod affected_tasks {
                 ])
             );
         }
+
+        #[tokio::test]
+        async fn deep_cycle() {
+            let workspace_graph = generate_workspace_graph("tasks").await;
+            let touched_files = FxHashSet::from_iter(["cycle/c.txt".into()]);
+
+            let mut tracker = AffectedTracker::new(&workspace_graph, &touched_files);
+            tracker.with_task_scopes(UpstreamScope::Deep, DownstreamScope::None);
+            tracker.track_tasks().unwrap();
+            let affected = tracker.build();
+
+            assert_eq!(
+                affected.tasks,
+                FxHashMap::from_iter([
+                    (
+                        Target::parse("cycle:global").unwrap(),
+                        create_state_from_file("cycle/c.txt")
+                    ),
+                    (Target::parse("cycle:c").unwrap(), {
+                        let mut state = create_state_from_file("cycle/c.txt");
+                        state.downstream.insert(Target::parse("cycle:b").unwrap());
+                        state
+                    }),
+                    (
+                        Target::parse("cycle:a").unwrap(),
+                        create_state_from_dependent("cycle:c")
+                    ),
+                    (
+                        Target::parse("cycle:b").unwrap(),
+                        create_state_from_dependent("cycle:a")
+                    ),
+                ])
+            );
+        }
     }
 
     mod task_downstream {
         use super::*;
-        use moon_test_utils2::pretty_assertions::assert_eq;
 
         #[tokio::test]
         async fn none() {
@@ -786,5 +870,38 @@ mod affected_tasks {
                 ])
             );
         }
+
+        #[tokio::test]
+        async fn deep_cycle() {
+            let workspace_graph = generate_workspace_graph("tasks").await;
+            let touched_files = FxHashSet::from_iter(["cycle/c.txt".into()]);
+
+            let mut tracker = AffectedTracker::new(&workspace_graph, &touched_files);
+            tracker.with_task_scopes(UpstreamScope::None, DownstreamScope::Deep);
+            tracker.track_tasks().unwrap();
+            let affected = tracker.build();
+
+            assert_eq!(
+                affected.tasks,
+                FxHashMap::from_iter([
+                    (
+                        Target::parse("cycle:global").unwrap(),
+                        create_state_from_file("cycle/c.txt")
+                    ),
+                    (
+                        Target::parse("cycle:c").unwrap(),
+                        create_state_from_file("cycle/c.txt")
+                    ),
+                    (
+                        Target::parse("cycle:b").unwrap(),
+                        create_state_from_dependency("cycle:c")
+                    ),
+                    (
+                        Target::parse("cycle:a").unwrap(),
+                        create_state_from_dependency("cycle:b")
+                    ),
+                ])
+            );
+        }
     }
 }