From f0949f90ddb46926b0010c14d938e64dc089b51f Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 29 Jul 2025 23:00:35 +0000 Subject: [PATCH] Fix Windows path length issue in memory storage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace agent.role with agent.id (UUID) in RAGStorage directory naming - UUIDs are guaranteed short (36 chars) and filesystem-safe - Remove unused _sanitize_role method since UUIDs don't need sanitization - Add comprehensive tests for Windows path length scenarios - Add test case for long agent roles in existing memory tests - Maintains backward compatibility while fixing Windows 260-char limit Fixes #3236 Co-Authored-By: João --- src/crewai/memory/storage/rag_storage.py | 7 +- tests/memory/short_term_memory_test.py | 38 +++++ tests/memory/test_windows_path_length.py | 190 +++++++++++++++++++++++ 3 files changed, 229 insertions(+), 6 deletions(-) create mode 100644 tests/memory/test_windows_path_length.py diff --git a/src/crewai/memory/storage/rag_storage.py b/src/crewai/memory/storage/rag_storage.py index eaa955fd3..096cc7c40 100644 --- a/src/crewai/memory/storage/rag_storage.py +++ b/src/crewai/memory/storage/rag_storage.py @@ -44,7 +44,7 @@ class RAGStorage(BaseRAGStorage): ): super().__init__(type, allow_reset, embedder_config, crew) agents = crew.agents if crew else [] - agents = [self._sanitize_role(agent.role) for agent in agents] + agents = [str(agent.id) for agent in agents] agents = "_".join(agents) self.agents = agents self.storage_file_name = self._build_storage_file_name(type, agents) @@ -74,11 +74,6 @@ class RAGStorage(BaseRAGStorage): ) logging.info(f"Collection found or created: {self.collection}") - def _sanitize_role(self, role: str) -> str: - """ - Sanitizes agent roles to ensure valid directory names. - """ - return role.replace("\n", "").replace(" ", "_").replace("/", "_") def _build_storage_file_name(self, type: str, file_name: str) -> str: """ diff --git a/tests/memory/short_term_memory_test.py b/tests/memory/short_term_memory_test.py index 020b3a94c..2c404b822 100644 --- a/tests/memory/short_term_memory_test.py +++ b/tests/memory/short_term_memory_test.py @@ -160,3 +160,41 @@ def test_save_and_search(short_term_memory): find = short_term_memory.search("test value", score_threshold=0.01)[0] assert find["context"] == memory.data, "Data value mismatch." assert find["metadata"]["agent"] == "test_agent", "Agent value mismatch." + + +def test_memory_with_long_agent_role(): + """Test that memory works correctly with very long agent roles.""" + very_long_role = ( + "Senior Equity Research Analyst specializing in corporate fundamentals and industry dynamics " + "with expertise in financial modeling, valuation techniques, and market analysis for " + "technology, healthcare, and consumer discretionary sectors, responsible for generating " + "comprehensive investment recommendations and detailed research reports" + ) + + agent = Agent( + role=very_long_role, + goal="Search relevant data and provide results", + backstory="You are a researcher at a leading tech think tank.", + tools=[], + verbose=True, + ) + + task = Task( + description="Perform a search on specific topics.", + expected_output="A list of relevant URLs based on the search query.", + agent=agent, + ) + + memory = ShortTermMemory(crew=Crew(agents=[agent], tasks=[task])) + + test_data = "Test memory data for long role agent" + test_metadata = {"task": "test_task"} + + memory.save( + value=test_data, + metadata=test_metadata, + agent=very_long_role, + ) + + results = memory.search("Test memory", score_threshold=0.01) + assert isinstance(results, list), "Search should return a list even with long agent roles" diff --git a/tests/memory/test_windows_path_length.py b/tests/memory/test_windows_path_length.py new file mode 100644 index 000000000..14740d059 --- /dev/null +++ b/tests/memory/test_windows_path_length.py @@ -0,0 +1,190 @@ +import pytest +from unittest.mock import patch +from crewai.agent import Agent +from crewai.crew import Crew +from crewai.memory.storage.rag_storage import RAGStorage +from crewai.task import Task +from crewai.utilities.constants import MAX_FILE_NAME_LENGTH + + +def test_long_agent_role_memory_storage(): + """Test that agents with very long roles don't exceed Windows path limits.""" + very_long_role = ( + "Senior Equity Research Analyst specializing in corporate fundamentals and industry dynamics " + "with expertise in financial modeling, valuation techniques, and market analysis for " + "technology, healthcare, and consumer discretionary sectors, responsible for generating " + "comprehensive investment recommendations and detailed research reports" + ) + + agent = Agent( + role=very_long_role, + goal="Analyze market trends and provide investment insights", + backstory="You are an experienced financial analyst with deep market knowledge.", + verbose=True, + ) + + task = Task( + description="Analyze the current market conditions.", + expected_output="A comprehensive market analysis report.", + agent=agent, + ) + + crew = Crew(agents=[agent], tasks=[task]) + + rag_storage = RAGStorage(type="short_term", crew=crew) + + assert len(rag_storage.storage_file_name) <= 260, ( + f"Storage path too long: {len(rag_storage.storage_file_name)} characters" + ) + + assert str(agent.id) in rag_storage.agents, ( + "Agent UUID should be used in storage path" + ) + + +def test_multiple_agents_with_long_roles(): + """Test that multiple agents with long roles create valid storage paths.""" + long_roles = [ + "Senior Investment Advisor specializing in equity portfolio strategy and client tailored recommendations", + "Financial Communications Specialist skilled in distilling complex analysis into concise client-facing investment reports", + "Registered Investment Advisor (RIA) specializing in equity portfolio strategy and client-tailored recommendations" + ] + + agents = [] + for i, role in enumerate(long_roles): + agent = Agent( + role=role, + goal=f"Goal for agent {i+1}", + backstory=f"Backstory for agent {i+1}", + verbose=True, + ) + agents.append(agent) + + tasks = [ + Task( + description=f"Task {i+1}", + expected_output=f"Output {i+1}", + agent=agent, + ) + for i, agent in enumerate(agents) + ] + + crew = Crew(agents=agents, tasks=tasks) + + rag_storage = RAGStorage(type="short_term", crew=crew) + + assert len(rag_storage.storage_file_name) <= 260, ( + f"Storage path too long: {len(rag_storage.storage_file_name)} characters" + ) + + for agent in agents: + assert str(agent.id) in rag_storage.agents, ( + f"Agent UUID {agent.id} should be in storage path" + ) + + +def test_memory_functionality_with_long_roles(): + """Test that memory save/search functionality works with long agent roles.""" + long_role = ( + "Senior Equity Research Analyst specializing in corporate fundamentals and industry dynamics " + "with expertise in financial modeling, valuation techniques, and market analysis" + ) + + agent = Agent( + role=long_role, + goal="Analyze market trends", + backstory="You are an experienced analyst.", + verbose=True, + ) + + task = Task( + description="Analyze market conditions.", + expected_output="Market analysis report.", + agent=agent, + ) + + crew = Crew(agents=[agent], tasks=[task]) + + rag_storage = RAGStorage(type="short_term", crew=crew) + + test_value = "Test memory content for long role agent" + test_metadata = {"task": "test_task", "agent": long_role} + + rag_storage.save(value=test_value, metadata=test_metadata) + + results = rag_storage.search(query="Test memory", limit=1, score_threshold=0.1) + + assert isinstance(results, list), "Search should return a list" + + +def test_backward_compatibility_short_roles(): + """Test that short agent roles still work correctly.""" + agent = Agent( + role="Researcher", + goal="Research topics", + backstory="You are a researcher.", + verbose=True, + ) + + task = Task( + description="Research a topic.", + expected_output="Research results.", + agent=agent, + ) + + crew = Crew(agents=[agent], tasks=[task]) + + rag_storage = RAGStorage(type="short_term", crew=crew) + + assert str(agent.id) in rag_storage.agents, ( + "Agent UUID should be used even for short roles" + ) + + assert len(rag_storage.storage_file_name) <= 260, ( + f"Storage path should be valid: {len(rag_storage.storage_file_name)} characters" + ) + + +def test_uuid_based_path_uniqueness(): + """Test that different agents with same role create different storage paths.""" + role = "Data Analyst" + + agent1 = Agent( + role=role, + goal="Analyze data", + backstory="You are an analyst.", + verbose=True, + ) + + agent2 = Agent( + role=role, + goal="Analyze data", + backstory="You are an analyst.", + verbose=True, + ) + + task1 = Task( + description="Analyze dataset 1.", + expected_output="Analysis 1.", + agent=agent1, + ) + + task2 = Task( + description="Analyze dataset 2.", + expected_output="Analysis 2.", + agent=agent2, + ) + + crew1 = Crew(agents=[agent1], tasks=[task1]) + crew2 = Crew(agents=[agent2], tasks=[task2]) + + storage1 = RAGStorage(type="short_term", crew=crew1) + storage2 = RAGStorage(type="short_term", crew=crew2) + + assert storage1.agents != storage2.agents, ( + "Different agents should create different storage paths even with same role" + ) + + assert str(agent1.id) in storage1.agents + assert str(agent2.id) in storage2.agents + assert str(agent1.id) != str(agent2.id)