Memory.forget() races pending background saves from remember_many(): forgotten content is resurrected after drain_writes()

[bossjoker1]

Description

Memory.forget() can return while a pending background save (submitted by remember_many()) still holds the very content being forgotten. When the background save lands afterwards, the "forgotten" content is resurrected and recall() returns it. forget() neither drains the pending-save queue nor takes _reset_lock, although the sibling destructive methods already do exactly that:

• recall() documents a read barrier and calls drain_writes() (unified_memory.py, recall — "Read barrier: wait for any pending background saves to finish").
• reset() / reset_all() hold _reset_lock and call drain_writes() before deleting.
• forget() (unified_memory.py:818) does neither; update() doesn't either.

The write path is genuinely asynchronous: remember_many() submits _background_encode_batch to a single-worker executor (_submit_save), and the docstring only promises a barrier for subsequent recall() calls. A caller that calls forget() immediately after remember_many() — the natural "correct the mistake" pattern — gets a return value of 0 ("nothing deleted") while the doomed write commits later, with no error and no way to observe the race.

Environment

• crewAI 1.15.20, pinned revision 143e902178a07d0f13f9db2308f983aaacaaf0f8 (editable install of lib/crewai)
• Memory + LanceDBStorage in a temporary directory; deterministic embedder fixture; no LLM is constructed (shallow recall, explicit scope/categories/metadata). No network.

Steps to Reproduce

Deterministic reproduction: the embedder blocks on a threading.Event while the background save is between submit and storage-write, so the interleaving is forced rather than lucky. Core schedule (full runnable script inline below):

from crewai.memory.storage.lancedb_storage import LanceDBStorage
from crewai.memory.unified_memory import Memory

memory = Memory(storage=LanceDBStorage(path=..., vector_dim=16), embedder=gated_embedder, llm="unused")

# 1. Background save submitted; the gated embedder holds it before the storage write.
memory.remember_many(
    ["Project Falcon ships on Friday. VG_FORGOTTEN_CANARY"],
    scope="/crew", categories=["projects"], metadata={"marker": "canary"},
    importance=0.5, source="s1",
)

# 2. Forget the same content while the save is still pending.
print(memory.forget(scope="/crew", metadata_filter={"marker": "canary"}))  # -> 0

# 3. Release the embedder; the background save now commits the "forgotten" content.
gated_embedder.gate.set()
memory.drain_writes()

# 4. Recall returns the forgotten content.
matches = memory.recall("Falcon", scope="/crew", depth="shallow", source="s1")
print([m.record.content for m in matches])
# ['Project Falcon ships on Friday. VG_FORGOTTEN_CANARY']

Observed output (pinned revision):

forget() returned:              0
recall after drain_writes():    ['Project Falcon ships on Friday. VG_FORGOTTEN_CANARY']

control A (sync remember -> forget):  forget returns 1; content is gone          # forget itself works
control B (remember_many -> reset):   recall returns []                          # reset() drains the queue

Expected Behavior

A destructive operation should observe the same write-queue discipline reset() already implements: forget() (and update()) should drain pending saves — or at least delete-then-drain-then-re-delete — before returning, so that a completed forget() call means the content can no longer appear in any later recall().

Suggested Fix

Add self.drain_writes() (and the _reset_lock mutual exclusion used by reset()) at the start of forget() and update(), mirroring the reset() implementation. Cost is bounded by the existing single-worker save pool.

Found during a correctness study of agent-memory lifecycle operations (delete/update vs. async derivation ordering). Happy to provide the full two-document schedule with per-step assertions.

[bunnysayzz]

I traced this to unified_memory.py.

forget() (line 818) calls self._storage.delete() directly without first draining the pending-save queue:

# line 844 — no drain_writes() before this
return self._storage.delete(
    scope_prefix=effective_scope,
    ...
)

recall() (line 711) already has the correct read barrier:

# Read barrier: wait for any pending background saves to finish
self.drain_writes()

forget() is missing the same barrier. A remember_many() background save submitted before forget() is called can land after the delete, resurrecting the forgotten content.

Fix: add self.drain_writes() at the top of forget(), before the scope resolution, mirroring recall(). One line. I'll open a PR with a regression test that fails on the current code (submit a background save, call forget() before it drains, then drain and recall — content should be gone but currently comes back).

[bunnysayzz]

Root cause confirmed in lib/crewai/src/crewai/memory/unified_memory.py.

forget() (line 818) calls self._storage.delete() at line 844 with no write barrier. recall() (line 711) already has self.drain_writes() before it touches storage. forget() is missing the same call, so a remember_many() background save submitted before forget() can land after the delete and resurrect the content.

Fix: one line — self.drain_writes() added at the top of forget(), before scope resolution, mirroring recall().

Three regression tests added in lib/crewai/tests/memory/test_memory_forget_drain.py — all 3 fail on the unpatched code, all 3 pass with the fix.

PR: #7291

[samvallad33]

Forget is not durable. You call it and a pending save brings the fact back. I tombstone the write and drain the queue first. Vestige recall cannot resurrect a forgotten fact.

https://github.com/samvallad33/vestige