mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-07-28 10:09:21 +00:00
fix: remove duplicate constraint injection in context, fix unused import
- Remove constraint text injection into context string (constraints are already rendered by Task.prompt() via the constraints field) - Remove unused MagicMock import from test file - Update tests to verify context is passed through unchanged Addresses review feedback from Cursor Bugbot and github-code-quality bot. Co-Authored-By: João <joao@crewai.com>
This commit is contained in:
@@ -56,8 +56,8 @@ class BaseAgentTool(BaseTool):
|
|||||||
Execute delegation to an agent with case-insensitive and whitespace-tolerant matching.
|
Execute delegation to an agent with case-insensitive and whitespace-tolerant matching.
|
||||||
|
|
||||||
When the original_task has constraints defined, they are automatically
|
When the original_task has constraints defined, they are automatically
|
||||||
propagated to the delegated task and appended to the context so that
|
propagated to the delegated Task object. The constraints are then
|
||||||
the worker agent is aware of all original requirements.
|
rendered by Task.prompt() so the worker agent sees them.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
agent_name: Name/role of the agent to delegate to (case-insensitive)
|
agent_name: Name/role of the agent to delegate to (case-insensitive)
|
||||||
@@ -122,7 +122,10 @@ class BaseAgentTool(BaseTool):
|
|||||||
|
|
||||||
selected_agent = agent[0]
|
selected_agent = agent[0]
|
||||||
try:
|
try:
|
||||||
# Propagate constraints from the original task to the delegated task
|
# Propagate constraints from the original task to the delegated task.
|
||||||
|
# Constraints are set on the Task object so that Task.prompt() renders
|
||||||
|
# them for the worker agent — no need to also inject them into `context`,
|
||||||
|
# which would cause duplication.
|
||||||
constraints: list[str] = []
|
constraints: list[str] = []
|
||||||
if self.original_task and self.original_task.constraints:
|
if self.original_task and self.original_task.constraints:
|
||||||
constraints = list(self.original_task.constraints)
|
constraints = list(self.original_task.constraints)
|
||||||
@@ -132,15 +135,6 @@ class BaseAgentTool(BaseTool):
|
|||||||
self.sanitize_agent_name(selected_agent.role),
|
self.sanitize_agent_name(selected_agent.role),
|
||||||
constraints,
|
constraints,
|
||||||
)
|
)
|
||||||
# Append constraints to context so the worker agent sees them
|
|
||||||
constraints_text = (
|
|
||||||
"\n\nTask Constraints (MUST be respected):\n"
|
|
||||||
+ "\n".join(f"- {c}" for c in constraints)
|
|
||||||
)
|
|
||||||
if context:
|
|
||||||
context = context + constraints_text
|
|
||||||
else:
|
|
||||||
context = constraints_text
|
|
||||||
|
|
||||||
task_with_assigned_agent = Task(
|
task_with_assigned_agent = Task(
|
||||||
description=task,
|
description=task,
|
||||||
|
|||||||
@@ -8,7 +8,7 @@ See: https://github.com/crewAIInc/crewAI/issues/5476
|
|||||||
"""
|
"""
|
||||||
|
|
||||||
import logging
|
import logging
|
||||||
from unittest.mock import MagicMock, patch
|
from unittest.mock import patch
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
@@ -156,17 +156,14 @@ class TestConstraintPropagationInDelegation:
|
|||||||
"Only frameworks available in Europe",
|
"Only frameworks available in Europe",
|
||||||
]
|
]
|
||||||
|
|
||||||
# The context should include the constraints
|
# Context should NOT be modified — constraints are rendered via Task.prompt()
|
||||||
assert "Task Constraints (MUST be respected):" in delegated_context
|
assert delegated_context == "Need a comprehensive list"
|
||||||
assert "- Only open-source frameworks" in delegated_context
|
|
||||||
assert "- Must be from 2024" in delegated_context
|
|
||||||
assert "- Only frameworks available in Europe" in delegated_context
|
|
||||||
|
|
||||||
@patch.object(Agent, "execute_task")
|
@patch.object(Agent, "execute_task")
|
||||||
def test_constraints_appended_to_existing_context(
|
def test_context_not_modified_by_constraints(
|
||||||
self, mock_execute, researcher, writer, task_with_constraints
|
self, mock_execute, researcher, writer, task_with_constraints
|
||||||
):
|
):
|
||||||
"""When context already exists, constraints are appended to it."""
|
"""Context is passed through unchanged; constraints live on the Task object."""
|
||||||
mock_execute.return_value = "result"
|
mock_execute.return_value = "result"
|
||||||
|
|
||||||
tools = AgentTools(agents=[researcher], task=task_with_constraints).tools()
|
tools = AgentTools(agents=[researcher], task=task_with_constraints).tools()
|
||||||
@@ -179,12 +176,13 @@ class TestConstraintPropagationInDelegation:
|
|||||||
)
|
)
|
||||||
|
|
||||||
mock_execute.assert_called_once()
|
mock_execute.assert_called_once()
|
||||||
|
delegated_task = mock_execute.call_args[0][0]
|
||||||
delegated_context = mock_execute.call_args[0][1]
|
delegated_context = mock_execute.call_args[0][1]
|
||||||
|
|
||||||
# Original context should still be there
|
# Context should be unchanged
|
||||||
assert delegated_context.startswith("Previous context here")
|
assert delegated_context == "Previous context here"
|
||||||
# Constraints should be appended
|
# Constraints should be on the task object
|
||||||
assert "Task Constraints (MUST be respected):" in delegated_context
|
assert len(delegated_task.constraints) == 3
|
||||||
|
|
||||||
@patch.object(Agent, "execute_task")
|
@patch.object(Agent, "execute_task")
|
||||||
def test_no_constraints_no_modification(
|
def test_no_constraints_no_modification(
|
||||||
@@ -230,13 +228,14 @@ class TestConstraintPropagationInDelegation:
|
|||||||
delegated_context = mock_execute.call_args[0][1]
|
delegated_context = mock_execute.call_args[0][1]
|
||||||
|
|
||||||
assert delegated_task.constraints == task_with_constraints.constraints
|
assert delegated_task.constraints == task_with_constraints.constraints
|
||||||
assert "Task Constraints (MUST be respected):" in delegated_context
|
# Context should be unchanged — constraints live on the task
|
||||||
|
assert delegated_context == "Need details"
|
||||||
|
|
||||||
@patch.object(Agent, "execute_task")
|
@patch.object(Agent, "execute_task")
|
||||||
def test_constraints_propagated_when_no_original_context(
|
def test_constraints_propagated_when_no_original_context(
|
||||||
self, mock_execute, researcher, writer, task_with_constraints
|
self, mock_execute, researcher, writer, task_with_constraints
|
||||||
):
|
):
|
||||||
"""When delegation has no context, constraints become the context."""
|
"""Even with empty context, constraints are on the task, not injected into context."""
|
||||||
mock_execute.return_value = "result"
|
mock_execute.return_value = "result"
|
||||||
|
|
||||||
tools = AgentTools(agents=[researcher], task=task_with_constraints).tools()
|
tools = AgentTools(agents=[researcher], task=task_with_constraints).tools()
|
||||||
@@ -249,10 +248,13 @@ class TestConstraintPropagationInDelegation:
|
|||||||
)
|
)
|
||||||
|
|
||||||
mock_execute.assert_called_once()
|
mock_execute.assert_called_once()
|
||||||
|
delegated_task = mock_execute.call_args[0][0]
|
||||||
delegated_context = mock_execute.call_args[0][1]
|
delegated_context = mock_execute.call_args[0][1]
|
||||||
|
|
||||||
# Empty string context means constraints text is appended to empty string
|
# Context should remain empty
|
||||||
assert "Task Constraints (MUST be respected):" in delegated_context
|
assert delegated_context == ""
|
||||||
|
# Constraints are on the task object
|
||||||
|
assert delegated_task.constraints == task_with_constraints.constraints
|
||||||
|
|
||||||
@patch.object(Agent, "execute_task")
|
@patch.object(Agent, "execute_task")
|
||||||
def test_delegation_without_original_task_works(
|
def test_delegation_without_original_task_works(
|
||||||
|
|||||||
Reference in New Issue
Block a user