mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-09-20 10:03:37 +00:00
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 <noreply@anthropic.com> Co-authored-by: Vidit Ostwal <110953813+Vidit-Ostwal@users.noreply.github.com>
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user