fix: detect circular task context dependencies#6441
Open
samrusani wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthroughAdds a Pydantic model validator to the ChangesCircular Context Dependency Detection
Sequence Diagram(s)sequenceDiagram
participant Developer
participant Crew
participant Validator as validate_context_no_circular_dependencies
participant Task
Developer->>Crew: create Crew(tasks=[...])
Crew->>Validator: run model validator (mode="after")
Validator->>Task: read task.context lists
loop DFS over task graph
Validator->>Validator: track visiting stack and path
alt cycle detected
Validator->>Crew: raise ValueError(dependency chain)
else no cycle
Validator->>Validator: mark task visited
end
end
Validator-->>Crew: return Self
Crew-->>Developer: Crew instance or ValueError
Compact metadata
Related issues
Related PRs
Suggested labels
Suggested reviewers
Poem 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Author
|
I do not have permission to apply repository labels from this fork PR, but this PR was authored with AI assistance. Please apply the required |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/crewai/src/crewai/crew.py (1)
838-878: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRecursive DFS can itself hit
RecursionErroron long (acyclic) context chains.The cycle-detection algorithm itself is correct, but
visit()recurses once per context edge. This PR's stated goal is to replaceRecursionError/hangs with a clearValueError(issue#6433), yet a sufficiently long non-cyclic dependency chain (approaching Python's default recursion limit, ~1000) would still raiseRecursionErrorhere — before the validator even reports the actual cycle, if one exists elsewhere. Converting to an iterative DFS (explicit stack) removes this residual risk without changing the detection semantics.♻️ Suggested iterative rewrite
`@model_validator`(mode="after") def validate_context_no_circular_dependencies(self) -> Self: """Validates that task context dependencies do not contain cycles.""" task_ids = {id(task) for task in self.tasks} - visiting: set[int] = set() visited: set[int] = set() - path: list[Task] = [] - - def visit(task: Task) -> None: - task_id = id(task) - if task_id in visited: - return - if task_id in visiting: - cycle_start = next( - index - for index, path_task in enumerate(path) - if id(path_task) == task_id - ) - cycle_tasks = [*path[cycle_start:], task] - cycle_descriptions = " -> ".join( - cycle_task.description for cycle_task in cycle_tasks - ) - raise ValueError( - "Task context dependencies contain a circular dependency: " - f"{cycle_descriptions}." - ) - - visiting.add(task_id) - path.append(task) - if isinstance(task.context, list): - for context_task in task.context: - if id(context_task) in task_ids: - visit(context_task) - path.pop() - visiting.remove(task_id) - visited.add(task_id) for task in self.tasks: - visit(task) + if id(task) in visited: + continue + path: list[Task] = [] + on_path: dict[int, int] = {} + stack: list[tuple[Task, Any]] = [(task, iter(task.context or []))] + on_path[id(task)] = 0 + path.append(task) + while stack: + current, it = stack[-1] + advanced = False + for context_task in it: + if id(context_task) not in task_ids: + continue + cid = id(context_task) + if cid in on_path: + cycle_tasks = [*path[on_path[cid] :], context_task] + cycle_descriptions = " -> ".join( + t.description for t in cycle_tasks + ) + raise ValueError( + "Task context dependencies contain a circular dependency: " + f"{cycle_descriptions}." + ) + if cid not in visited: + on_path[cid] = len(path) + path.append(context_task) + stack.append( + (context_task, iter(context_task.context or [])) + ) + advanced = True + break + if not advanced: + finished, _ = stack.pop() + path.pop() + del on_path[id(finished)] + visited.add(id(finished)) return self🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/src/crewai/crew.py` around lines 838 - 878, The recursive DFS in validate_context_no_circular_dependencies on Crew can still raise RecursionError for long acyclic task.context chains before it reports the intended ValueError. Replace the nested visit() recursion with an iterative DFS using an explicit stack while preserving the current visiting/visited/path cycle-detection behavior and the same circular-dependency error message. Keep the logic scoped around Crew.tasks and Task.context so the validator still detects cycles without depending on Python recursion depth.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@lib/crewai/src/crewai/crew.py`:
- Around line 838-878: The recursive DFS in
validate_context_no_circular_dependencies on Crew can still raise RecursionError
for long acyclic task.context chains before it reports the intended ValueError.
Replace the nested visit() recursion with an iterative DFS using an explicit
stack while preserving the current visiting/visited/path cycle-detection
behavior and the same circular-dependency error message. Keep the logic scoped
around Crew.tasks and Task.context so the validator still detects cycles without
depending on Python recursion depth.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0ede7cde-a453-4e47-a96e-433d7e8470cc
📒 Files selected for processing (2)
lib/crewai/src/crewai/crew.pylib/crewai/tests/test_crew.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Crewis constructedValueErrorinstead of a runtime recursion/hangTask 1 -> Task 2 -> Task 3 -> Task 1Fixes #6433
Testing
uv run pytest lib/crewai/tests/test_crew.py::test_context_no_circular_dependencies lib/crewai/tests/test_crew.py::test_context_no_future_tasks -quv run ruff check lib/crewai/src/crewai/crew.py lib/crewai/tests/test_crew.pyuv run ruff format lib/crewai/src/crewai/crew.py lib/crewai/tests/test_crew.pygit diff --checkNotes
Authored with AI assistance. Please keep/apply the
llm-generatedlabel per the contribution guide.