mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-07-06 23:49:24 +00:00
fix: wire planning-mode native path and harden artifact helpers
- Wire resolve_artifact_handles/store_if_artifact into agent_utils.execute_single_native_tool_call, the native tool path used by StepExecutor (Plan-and-Execute mode); previously a FileArtifact produced or consumed there bypassed the bypass and leaked raw bytes. - Fold the crew/agent.crew fallback into artifact_scope_id(crew, task, agent) so all four execution paths derive the cleanup scope identically instead of repeating 'self.crew or getattr(self.agent, ...)'. - Sanitize ']' and newlines (not just quotes) in the placeholder so an odd filename can't break the bracketed attribute line; extend _human_size with TB/PB.
This commit is contained in:
@@ -869,9 +869,7 @@ class CrewAgentExecutor(BaseAgentExecutor):
|
||||
store_if_artifact,
|
||||
)
|
||||
|
||||
scope_id = artifact_scope_id(
|
||||
self.crew or getattr(self.agent, "crew", None), self.task
|
||||
)
|
||||
scope_id = artifact_scope_id(self.crew, self.task, self.agent)
|
||||
|
||||
args_dict, parse_error = parse_tool_call_args(
|
||||
func_args, func_name, call_id, original_tool
|
||||
|
||||
@@ -1767,9 +1767,7 @@ class AgentExecutor(Flow[AgentExecutorState], BaseAgentExecutor):
|
||||
return parse_error
|
||||
args_dict: dict[str, Any] = parsed_args or {}
|
||||
|
||||
scope_id = artifact_scope_id(
|
||||
self.crew or getattr(self.agent, "crew", None), self.task
|
||||
)
|
||||
scope_id = artifact_scope_id(self.crew, self.task, self.agent)
|
||||
|
||||
# Get agent_key for event tracking
|
||||
agent_key = getattr(self.agent, "key", "unknown") if self.agent else "unknown"
|
||||
|
||||
@@ -78,9 +78,10 @@ class FileArtifact:
|
||||
|
||||
def _placeholder(self, handle: str) -> str:
|
||||
"""Build the model-facing text that stands in for the bytes."""
|
||||
# Quote-escape so a filename/mime containing `"` can't break the brackets.
|
||||
filename = self.filename.replace('"', "'")
|
||||
mime_type = self.mime_type.replace('"', "'")
|
||||
# Neutralize characters that would break the single-line bracketed
|
||||
# attribute list (quotes, the closing bracket, newlines).
|
||||
filename = _sanitize_attr(self.filename)
|
||||
mime_type = _sanitize_attr(self.mime_type)
|
||||
return (
|
||||
f'[FileArtifact filename="{filename}" '
|
||||
f'mime_type="{mime_type}" size={_human_size(self.size_bytes)} '
|
||||
@@ -224,24 +225,38 @@ def clear_artifact_scope(scope_id: Any) -> None:
|
||||
_store.clear_scope(scope_id)
|
||||
|
||||
|
||||
def artifact_scope_id(crew: Any | None, task: Any | None) -> Any | None:
|
||||
def artifact_scope_id(
|
||||
crew: Any | None = None,
|
||||
task: Any | None = None,
|
||||
agent: Any | None = None,
|
||||
) -> Any | None:
|
||||
"""Pick the execution id used to scope a tool's file artifacts for cleanup.
|
||||
|
||||
Prefer the crew id -- it matches the id ``Crew`` passes to
|
||||
:func:`clear_artifact_scope` when a run ends -- then the task id, then
|
||||
``None`` (TTL-only cleanup). Centralized so the two executors and the ReAct
|
||||
path can't drift in how they derive the scope.
|
||||
:func:`clear_artifact_scope` when a run ends -- falling back to the agent's
|
||||
crew, then the task id, then ``None`` (TTL-only cleanup). Centralized, and
|
||||
given the agent fallback, so every tool-execution path derives the scope the
|
||||
same way and can't drift.
|
||||
"""
|
||||
if crew is None:
|
||||
crew = getattr(agent, "crew", None)
|
||||
crew_id = getattr(crew, "id", None)
|
||||
if crew_id is not None:
|
||||
return crew_id
|
||||
return getattr(task, "id", None)
|
||||
|
||||
|
||||
def _sanitize_attr(text: str) -> str:
|
||||
"""Strip characters that would break the bracketed placeholder display."""
|
||||
return (
|
||||
text.replace('"', "'").replace("]", ")").replace("\n", " ").replace("\r", " ")
|
||||
)
|
||||
|
||||
|
||||
def _human_size(size_bytes: int) -> str:
|
||||
size = float(size_bytes)
|
||||
for unit in ("B", "KB", "MB", "GB"):
|
||||
if size < 1024 or unit == "GB":
|
||||
for unit in ("B", "KB", "MB", "GB", "TB", "PB"):
|
||||
if size < 1024 or unit == "PB":
|
||||
return f"{int(size)} {unit}" if unit == "B" else f"{size:.1f} {unit}"
|
||||
size /= 1024
|
||||
return f"{size:.1f} GB"
|
||||
return f"{size:.1f} PB"
|
||||
|
||||
@@ -687,7 +687,7 @@ class ToolUsage:
|
||||
@property
|
||||
def _artifact_scope_id(self) -> Any | None:
|
||||
"""Execution id used to scope out-of-band file artifacts for cleanup."""
|
||||
return artifact_scope_id(getattr(self.agent, "crew", None), self.task)
|
||||
return artifact_scope_id(task=self.task, agent=self.agent)
|
||||
|
||||
def _format_result(self, result: Any) -> str:
|
||||
from crewai.tools.file_artifact import store_if_artifact
|
||||
|
||||
@@ -27,6 +27,11 @@ from crewai.agents.parser import (
|
||||
from crewai.llms.base_llm import BaseLLM, call_stop_override
|
||||
from crewai.tools import BaseTool as CrewAITool
|
||||
from crewai.tools.base_tool import BaseTool
|
||||
from crewai.tools.file_artifact import (
|
||||
artifact_scope_id,
|
||||
resolve_artifact_handles,
|
||||
store_if_artifact,
|
||||
)
|
||||
from crewai.tools.structured_tool import CrewStructuredTool
|
||||
from crewai.tools.tool_types import ToolResult
|
||||
from crewai.utilities.errors import AgentRepositoryError
|
||||
@@ -1416,6 +1421,7 @@ def execute_single_native_tool_call(
|
||||
args_dict = func_args
|
||||
|
||||
agent_key = getattr(agent, "key", "unknown") if agent else "unknown"
|
||||
scope_id = artifact_scope_id(crew, task, agent)
|
||||
|
||||
original_tool: BaseTool | None = None
|
||||
for tool in original_tools:
|
||||
@@ -1430,6 +1436,7 @@ def execute_single_native_tool_call(
|
||||
if tools_handler and tools_handler.cache:
|
||||
cached_result = tools_handler.cache.read(tool=func_name, input=input_str)
|
||||
if cached_result is not None:
|
||||
cached_result = store_if_artifact(cached_result, scope_id)
|
||||
result = (
|
||||
str(cached_result)
|
||||
if not isinstance(cached_result, str)
|
||||
@@ -1481,7 +1488,8 @@ def execute_single_native_tool_call(
|
||||
if func_name in available_functions:
|
||||
try:
|
||||
tool_func = available_functions[func_name]
|
||||
raw_result = tool_func(**args_dict)
|
||||
invoke_args = resolve_artifact_handles(args_dict) if args_dict else {}
|
||||
raw_result = tool_func(**invoke_args)
|
||||
|
||||
if tools_handler and tools_handler.cache:
|
||||
should_cache = True
|
||||
@@ -1494,6 +1502,7 @@ def execute_single_native_tool_call(
|
||||
tool=func_name, input=input_str, output=raw_result
|
||||
)
|
||||
|
||||
raw_result = store_if_artifact(raw_result, scope_id)
|
||||
result = (
|
||||
str(raw_result) if not isinstance(raw_result, str) else raw_result
|
||||
)
|
||||
|
||||
@@ -75,6 +75,15 @@ class TestStoreArtifact:
|
||||
assert 'filename="a\'.pptx"' in placeholder
|
||||
assert _HANDLE.search(placeholder) is not None
|
||||
|
||||
def test_placeholder_neutralizes_bracket_and_newlines(self) -> None:
|
||||
artifact = FileArtifact(data=b"x", filename="a]b\nc.bin")
|
||||
placeholder = store_artifact(artifact)
|
||||
first_line = placeholder.splitlines()[0]
|
||||
# The closing bracket and newline can't appear inside the attributes,
|
||||
# so the bracketed segment stays a single, well-formed line.
|
||||
assert first_line.count("]") == 1 and first_line.endswith("]")
|
||||
assert _HANDLE.search(placeholder) is not None
|
||||
|
||||
|
||||
class TestArtifactScopeId:
|
||||
class _Obj:
|
||||
@@ -93,6 +102,18 @@ class TestArtifactScopeId:
|
||||
def test_none_when_neither_present(self) -> None:
|
||||
assert artifact_scope_id(None, None) is None
|
||||
|
||||
def test_falls_back_to_agent_crew(self) -> None:
|
||||
# Native executors may have crew=None while the agent carries the crew;
|
||||
# the helper must still resolve the crew id so cleanup scopes align.
|
||||
agent = self._Obj(None)
|
||||
agent.crew = self._Obj("crew-from-agent")
|
||||
assert artifact_scope_id(None, self._Obj("task"), agent) == "crew-from-agent"
|
||||
|
||||
def test_explicit_crew_beats_agent_crew(self) -> None:
|
||||
agent = self._Obj(None)
|
||||
agent.crew = self._Obj("agent-crew")
|
||||
assert artifact_scope_id(self._Obj("direct-crew"), None, agent) == "direct-crew"
|
||||
|
||||
|
||||
class TestResolveArtifactHandles:
|
||||
def test_exact_handle_resolves_to_base64(self) -> None:
|
||||
|
||||
Reference in New Issue
Block a user