Fix duplicated cycle detection with consistent error handling

cli/localci/core/queue.py

@@ -105,24 +105,35 @@ def add_job(self, job_id: str, needs: list[str]) -> None:
             self._reverse[dep].append(job_id)
 
     def resolve(self) -> list[str]:
+        """Return a topological order of registered jobs.
+
+        Dependencies listed in ``needs`` but never passed to :meth:`add_job`
+        are still visited with an empty edge list and included in the order.
+        Callers (e.g. ``Workflow.dependency_order``) must register a complete
+        graph.
+        """
         visited: set[str] = set()
         temp_visited: set[str] = set()
         order: list[str] = []
 
-        def visit(jid: str) -> None:
+        def visit(jid: str, path: list[str]) -> None:
             if jid in temp_visited:
-                raise CyclicDependencyError(jid)
+                idx = path.index(jid)
+                cycle = path[idx:] + [jid]
+                raise CyclicDependencyError(cycle)
             if jid in visited:
                 return
             temp_visited.add(jid)
+            path.append(jid)
             for dep in self._graph.get(jid, []):
-                visit(dep)
+                visit(dep, path)
+            path.pop()
             temp_visited.discard(jid)
             visited.add(jid)
             order.append(jid)
 
         for jid in self._graph:
-            visit(jid)
+            visit(jid, [])
         return order
 
     def get_dependencies(self, job_id: str) -> list[str]:

cli/localci/core/workflow.py

@@ -15,6 +15,7 @@
 from pathlib import Path
 from typing import Any, Optional
 
+from localci.core.queue import DependencyResolver
 from localci.errors import (
     LocalCIError,
     MissingFieldError,
@@ -302,30 +303,15 @@ def platform_summary(self) -> dict[Platform, int]:
 
     def dependency_order(self) -> list[str]:
         """Topological sort of jobs by dependencies."""
-        visited: set[str] = set()
-        in_progress: set[str] = set()
-        order: list[str] = []
-
-        def visit(job_id: str) -> None:
-            if job_id in visited:
-                return
-            if job_id not in self.jobs:
-                logger.warning("Dependency '%s' not found in jobs", job_id)
-                return
-            if job_id in in_progress:
-                logger.warning("Circular dependency detected involving '%s'", job_id)
-                return
-            in_progress.add(job_id)
-            visited.add(job_id)
-            for dep in self.jobs[job_id].needs:
-                visit(dep)
-            in_progress.discard(job_id)
-            order.append(job_id)
-
-        for job_id in self.jobs:
-            visit(job_id)
-
-        return order
+        resolver = DependencyResolver()
+        job_ids = set(self.jobs)
+        for job_id, job in self.jobs.items():
+            needs = [d for d in job.needs if d in job_ids]
+            for dep in job.needs:
+                if dep not in job_ids:
+                    logger.warning("Dependency '%s' not found in jobs", dep)
+            resolver.add_job(job_id, needs)
+        return resolver.resolve()
 
 
 # =====================================================================

cli/localci/errors.py

@@ -188,11 +188,27 @@ def __init__(self, entry: dict[str, Any], detail: str) -> None:
 
 
 class CyclicDependencyError(WorkflowError):
-    """Circular dependency detected in job graph."""
+    """Circular dependency detected in job graph.
 
-    def __init__(self, job_id: str) -> None:
-        super().__init__(f"Cyclic dependency detected involving job: {job_id}")
-        self.job_id = job_id
+    Attributes
+    ----------
+    cycle:
+        Ordered job IDs forming the loop; first and last elements are equal
+        (e.g. ``["a", "b", "a"]``).
+    job_id:
+        First job in the cycle, kept for backward compatibility (``cycle[0]``).
+    """
+
+    def __init__(self, cycle: list[str]) -> None:
+        if not isinstance(cycle, list):
+            raise TypeError(
+                f"cycle must be a list[str], got {type(cycle).__name__!r}; "
+                "pass an ordered path such as ['a', 'b', 'a'], not a single job_id string"
+            )
+        self.cycle = list(cycle)
+        self.job_id = self.cycle[0] if self.cycle else ""
+        path = " -> ".join(self.cycle)
+        super().__init__(f"Cyclic dependency detected: {path}")
 
 
 # ---------------------------------------------------------------------------

cli/tests/test_errors.py

@@ -168,6 +168,18 @@ def test_workflow_parse_error_attributes(self, tmp_path):
         assert "unexpected key" in str(exc)
         assert str(p) in str(exc)
 
+    def test_cyclic_dependency_error_attributes(self):
+        cycle = ["a", "b", "a"]
+        exc = CyclicDependencyError(cycle)
+        assert exc.cycle == cycle
+        assert exc.job_id == "a"
+        assert "a -> b -> a" in str(exc)
+        assert isinstance(exc, WorkflowError)
+
+    def test_cyclic_dependency_error_rejects_string(self):
+        with pytest.raises(TypeError, match="cycle must be a list"):
+            CyclicDependencyError("ab")
+
 
 # ---------------------------------------------------------------------------
 # Backward-compatibility: caught by built-in types

cli/tests/test_queue.py

@@ -118,8 +118,13 @@ def test_cyclic_dependency(self):
         resolver = DependencyResolver()
         resolver.add_job("a", ["b"])
         resolver.add_job("b", ["a"])
-        with pytest.raises(CyclicDependencyError):
+        with pytest.raises(CyclicDependencyError) as exc_info:
             resolver.resolve()
+        exc = exc_info.value
+        assert exc.cycle == ["a", "b", "a"]
+        assert exc.cycle[0] == exc.cycle[-1]
+        assert exc.job_id == "a"
+        assert "a -> b -> a" in str(exc)
 
     def test_all_dependencies_met(self):
         resolver = DependencyResolver()

cli/tests/test_workflow.py

@@ -11,6 +11,7 @@
 
 import pytest
 
+from localci.errors import CyclicDependencyError
 from localci.core.workflow import (
     BuildSystem,
     BuildVariant,
@@ -350,6 +351,34 @@ def test_changelog_needs_build(self, sample_workflow):
         changelog = sample_workflow.jobs["changelog"]
         assert "build" in changelog.needs
 
+    def test_cyclic_dependency_raises(self, tmp_path):
+        workflow = Workflow(
+            name="cyclic",
+            file_path=tmp_path / "cyclic.yml",
+            events=["push"],
+            jobs={
+                "a": Job(
+                    id="a",
+                    name="A",
+                    runs_on="ubuntu-latest",
+                    needs=["b"],
+                ),
+                "b": Job(
+                    id="b",
+                    name="B",
+                    runs_on="ubuntu-latest",
+                    needs=["a"],
+                ),
+            },
+        )
+        with pytest.raises(CyclicDependencyError) as exc_info:
+            workflow.dependency_order()
+        exc = exc_info.value
+        assert exc.cycle == ["a", "b", "a"]
+        assert exc.cycle[0] == exc.cycle[-1]
+        assert exc.job_id == "a"
+        assert "a -> b -> a" in str(exc)
+
 
 # =====================================================================
 # Container info