From c6ff78650e442c9e0ebfca23d9afe12775fa6e5e Mon Sep 17 00:00:00 2001 From: Roli Bosch Date: Tue, 15 Sep 2026 07:20:49 -0400 Subject: [PATCH] fix(memory): honor read_only on update() and recall() access times (#7369) `Memory.read_only` was enforced only on `remember()` and `remember_many()`. Two other paths still mutated the backing store: - `update()` re-embedded the supplied content and wrote the record back. - `recall()` refreshed `last_accessed` through `touch_records()`, so simply reading a read-only memory left a persistent trace. Both now respect the flag, so a read-only Memory leaves stored records unchanged. `update()` returns the existing record untouched rather than raising, matching the silent no-op behaviour of `remember()`. Explicit deletion through `forget()`/`reset()` is deliberately unaffected. Claude-Session: https://claude.ai/code/session_01HkDjVYVzHFEj5re9B8JH9p Co-authored-by: Claude Opus 5 Co-authored-by: Vidit Ostwal <110953813+Vidit-Ostwal@users.noreply.github.com> --- .../src/crewai/memory/unified_memory.py | 14 ++++- .../tests/memory/test_unified_memory.py | 59 +++++++++++++++++++ 2 files changed, 70 insertions(+), 3 deletions(-) diff --git a/lib/crewai/src/crewai/memory/unified_memory.py b/lib/crewai/src/crewai/memory/unified_memory.py index dcd5383ce..c0654a2ba 100644 --- a/lib/crewai/src/crewai/memory/unified_memory.py +++ b/lib/crewai/src/crewai/memory/unified_memory.py @@ -147,7 +147,12 @@ class Memory(BaseModel): ) read_only: bool = Field( default=False, - description="If True, remember() and remember_many() are silent no-ops.", + description=( + "If True, stored records are left unchanged: remember() and " + "remember_many() store nothing, update() leaves the record " + "untouched, and recall() does not refresh access times. Explicit " + "deletion through forget()/reset() is unaffected." + ), ) root_scope: str | None = Field( default=None, @@ -781,7 +786,7 @@ class Memory(BaseModel): ) results = flow.state.final_results - if results: + if results and not self.read_only: try: touch = getattr(self._storage, "touch_records", None) if touch is not None: @@ -869,7 +874,8 @@ class Memory(BaseModel): importance: New importance score. Returns: - The updated MemoryRecord. + The updated MemoryRecord, or the unchanged record when ``read_only`` + is set. Raises: ValueError: If the record is not found. @@ -877,6 +883,8 @@ class Memory(BaseModel): existing = self._storage.get_record(record_id) if existing is None: raise ValueError(f"Record not found: {record_id}") + if self.read_only: + return existing now = datetime.utcnow() updates: dict[str, Any] = {"last_accessed": now} if content is not None: diff --git a/lib/crewai/tests/memory/test_unified_memory.py b/lib/crewai/tests/memory/test_unified_memory.py index 65363efad..a1c4e989d 100644 --- a/lib/crewai/tests/memory/test_unified_memory.py +++ b/lib/crewai/tests/memory/test_unified_memory.py @@ -335,6 +335,65 @@ def test_memory_slice_remember_is_noop_when_read_only(tmp_path: Path, mock_embed assert mem.list_records() == [] +def test_update_is_noop_when_read_only(tmp_path: Path, mock_embedder: MagicMock) -> None: + """A read-only Memory leaves stored records untouched when update() is called.""" + from crewai.memory.unified_memory import Memory + + mem = Memory(storage=str(tmp_path / "db8"), llm=MagicMock(), embedder=mock_embedder) + record = mem.remember( + "original", scope="/a", categories=[], importance=0.5, metadata={} + ) + assert record is not None + + mem.read_only = True + returned = mem.update(record.id, content="rewritten", importance=0.9) + + assert returned.content == "original" + assert returned.importance == 0.5 + stored = mem.list_records() + assert [(r.content, r.importance) for r in stored] == [("original", 0.5)] + + +def test_update_still_writes_when_not_read_only( + tmp_path: Path, mock_embedder: MagicMock +) -> None: + """The read-only guard does not change update() for a writable Memory.""" + from crewai.memory.unified_memory import Memory + + mem = Memory(storage=str(tmp_path / "db9"), llm=MagicMock(), embedder=mock_embedder) + record = mem.remember( + "original", scope="/a", categories=[], importance=0.5, metadata={} + ) + assert record is not None + + returned = mem.update(record.id, content="rewritten", importance=0.9) + + assert returned.content == "rewritten" + assert returned.importance == 0.9 + stored = mem.list_records() + assert [(r.content, r.importance) for r in stored] == [("rewritten", 0.9)] + + +def test_recall_does_not_refresh_access_time_when_read_only( + tmp_path: Path, mock_embedder: MagicMock +) -> None: + """Recall against a read-only Memory leaves last_accessed untouched.""" + from crewai.memory.unified_memory import Memory + + mem = Memory(storage=str(tmp_path / "db10"), llm=MagicMock(), embedder=mock_embedder) + mem.remember("alpha", scope="/a", categories=[], importance=0.5, metadata={}) + before = mem.list_records()[0].last_accessed + + mem.read_only = True + assert mem.recall("alpha", scope="/a", limit=5, depth="shallow") + assert mem.list_records()[0].last_accessed == before + + # Positive control: a writable Memory still refreshes the access time. + mem.read_only = False + assert mem.recall("alpha", scope="/a", limit=5, depth="shallow") + assert mem.list_records()[0].last_accessed > before + + def test_flow_has_default_memory() -> None: