Enhance memory reset functionality and JSON crew handling - #6195
Merged
Conversation
- Added `reset_all` method to the `Memory` class to reset the entire memory store, ignoring `root_scope`. - Updated the `Crew` class to utilize `reset_all` when resetting memory. - Enhanced the `_reset_flow_memory` function to check for `Memory` instances and call `reset_all` accordingly. - Introduced helper functions to load JSON crew configurations and handle project declarations, improving the reset command's flexibility. - Added tests to validate the new JSON crew memory reset behavior and ensure proper handling of declared flow projects.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthrough
ChangesMemory Reset API and Project-Aware Reset Command
Sequence Diagram(s)sequenceDiagram
participant Client
participant Crew
participant Memory
participant ResetLock
participant Storage
Client->>Crew: reset_memories(command_type="memory")
Crew->>Memory: reset_all()
Memory->>ResetLock: acquire()
Memory->>Memory: drain_writes()
Memory->>Storage: reset(scope_prefix=None)
Memory->>ResetLock: release()
par Concurrent background save
Client->>Memory: _submit_save()
Memory->>ResetLock: acquire() [waits for reset]
Memory->>Storage: submit/sync
Memory->>ResetLock: release()
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6c185d2. Configure here.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
lib/crewai/tests/cli/test_cli.py (1)
182-191: ⚡ Quick winAdd a focused test for the new
Memory-specificreset_all()path.Current flow reset tests exercise the generic
.reset()path, but not the newisinstance(mem, Memory) -> reset_all()branch. A direct test would lock in the behavior introduced in this PR.Example test shape
+from crewai.memory.unified_memory import Memory ... +def test_reset_flow_unified_memory_uses_reset_all(runner): + mock_flow = mock.Mock() + mock_flow.name = "TestFlow" + mem = mock.Mock(spec=Memory) + mock_flow.memory = mem + + with mock.patch("crewai.utilities.reset_memories.get_crews", return_value=[]), \ + mock.patch("crewai.utilities.reset_memories.get_flows", return_value=[mock_flow]), \ + mock.patch("crewai.utilities.reset_memories._get_json_crew", return_value=None): + runner.invoke(reset_memories, ["-m"]) + + mem.reset_all.assert_called_once() + mem.reset.assert_not_called()Also applies to: 253-266
🤖 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/tests/cli/test_cli.py` around lines 182 - 191, The current tests test_reset_flow_memory and test_reset_flow_all_memories only exercise the generic .reset() path but do not directly test the new isinstance(mem, Memory) -> reset_all() code branch. Add a focused test that specifically verifies the Memory-specific reset_all() path is called when resetting memories. Create a new test that mocks a Memory instance and asserts that reset_all() is invoked instead of the generic reset() method. Apply the same test addition to the similar code section referenced at lines 253-266 to ensure comprehensive coverage of the new Memory-specific behavior.
🤖 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.
Inline comments:
In `@lib/crewai/src/crewai/memory/unified_memory.py`:
- Around line 993-1004: The reset and reset_all methods are not atomic against
concurrent background saves because drain_writes() only waits for a snapshot of
pending futures at that moment, allowing new saves to be submitted between the
drain_writes() call and the self._storage.reset() call, which can repopulate the
storage after reset. To fix this, implement a synchronization mechanism that
prevents new writes from being submitted during the reset operation, such as
acquiring a lock before calling drain_writes() in both the reset method and
reset_all method, and releasing it only after self._storage.reset() completes,
ensuring no writes can arrive between draining and resetting.
In `@lib/crewai/src/crewai/utilities/reset_memories.py`:
- Around line 66-67: The load_crew function call in the crew reset operation can
raise exceptions due to invalid JSON/JSONC or schema validation issues, causing
the entire reset command to abort and preventing other crews/flows from being
processed. Wrap the load_crew(crew_path) call at line 66 in a try-except block
to catch exceptions gracefully, log the error appropriately, and continue
processing remaining crews instead of aborting. Apply the same error handling
pattern to the similar code block at lines 93-96 to ensure consistent behavior
throughout the reset operation.
---
Nitpick comments:
In `@lib/crewai/tests/cli/test_cli.py`:
- Around line 182-191: The current tests test_reset_flow_memory and
test_reset_flow_all_memories only exercise the generic .reset() path but do not
directly test the new isinstance(mem, Memory) -> reset_all() code branch. Add a
focused test that specifically verifies the Memory-specific reset_all() path is
called when resetting memories. Create a new test that mocks a Memory instance
and asserts that reset_all() is invoked instead of the generic reset() method.
Apply the same test addition to the similar code section referenced at lines
253-266 to ensure comprehensive coverage of the new Memory-specific behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 096cebce-c64c-459b-8e80-8e3fc35e33bf
📒 Files selected for processing (4)
lib/crewai/src/crewai/crew.pylib/crewai/src/crewai/memory/unified_memory.pylib/crewai/src/crewai/utilities/reset_memories.pylib/crewai/tests/cli/test_cli.py
Collaborator
Author
|
Addressed the CodeRabbit summary nitpick in d2e5775 as well: added |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/crewai/tests/memory/test_unified_memory.py (1)
981-1002: ⚡ Quick winAssert the save body cannot run before reset completes.
This test currently proves
_submit_save()does not return during reset, but it would miss a regression where the future is submitted before taking_reset_lockand only returns later. Wait for the submitted save and assert"save"occurs after"reset-end".Strengthen the concurrency assertion
reset_thread.join(timeout=2) submit_thread.join(timeout=2) + mem.drain_writes() assert not reset_thread.is_alive() assert not submit_thread.is_alive() assert order.index("reset-end") < order.index("submit-returned") + assert order.index("reset-end") < order.index("save")🤖 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/tests/memory/test_unified_memory.py` around lines 981 - 1002, The test currently only verifies that the _submit_save() call returns after reset completes, but does not verify that the actual save body executes after reset. This misses a regression where the future is submitted before acquiring _reset_lock. Add code to wait for the submitted save operation (the lambda that appends "save") to complete execution, then add an assertion to verify that the "save" entry appears after "reset-end" in the order list, similar to the existing assertion for "submit-returned".
🤖 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/tests/memory/test_unified_memory.py`:
- Around line 981-1002: The test currently only verifies that the _submit_save()
call returns after reset completes, but does not verify that the actual save
body executes after reset. This misses a regression where the future is
submitted before acquiring _reset_lock. Add code to wait for the submitted save
operation (the lambda that appends "save") to complete execution, then add an
assertion to verify that the "save" entry appears after "reset-end" in the order
list, similar to the existing assertion for "submit-returned".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ae6f7c7c-0f19-46db-b8c7-d8144105cb53
📒 Files selected for processing (6)
lib/crewai/src/crewai/memory/unified_memory.pylib/crewai/src/crewai/utilities/reset_memories.pylib/crewai/tests/cli/test_cli.pylib/crewai/tests/memory/test_dimension_mismatch.pylib/crewai/tests/memory/test_unified_memory.pylib/crewai/tests/test_crew.py
✅ Files skipped from review due to trivial changes (1)
- lib/crewai/tests/memory/test_dimension_mismatch.py
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/crewai/src/crewai/utilities/reset_memories.py
alex-clawd
approved these changes
Jun 17, 2026
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
2 participants
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.
reset_allmethod to theMemoryclass to reset the entire memory store, ignoringroot_scope.Crewclass to utilizereset_allwhen resetting memory._reset_flow_memoryfunction to check forMemoryinstances and callreset_allaccordingly.Note
Medium Risk
Changes persistent memory wipe semantics and concurrent save behavior during reset; incorrect locking could cause data loss or stuck saves, though coverage targets the race.
Overview
Improves unified memory reset so CLI and crew/flow paths clear the whole backing store (not only
root_scope), and coordinates resets with background saves. Bumps the optional litellm extra to>=1.84.0,<1.85(lockfile updated).Memory: Adds
reset_all()(full storage wipe viascope_prefix=None).reset()andreset_all()now take an_reset_lock, calldrain_writes()first, and_submit_saveruns under the same lock so new saves cannot slip in mid-reset. Crew and flow reset paths callreset_all()forMemoryinstances.CLI
reset-memories: Discovers JSON-first crews fromcrew.jsonc(viaload_crew) and appends them to the crew list; skips that path whenpyproject.tomldeclarestype = "flow". Invalid JSON crews log a skip message without blocking classic crews.Tests cover full reset for crews/flows, reset/save ordering, JSON crew scenarios, and
reset_allafter embedding dimension mismatch.Reviewed by Cursor Bugbot for commit 65f357e. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests
Chores
litellmversion range.