Skip to content

fix(memory): isolate ingested events and returned search content #7566

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hackermengzhi wants to merge 2 commits into
base: main
Choose a base branch
from
+73 −3
Open

fix(memory): isolate ingested events and returned search content #7566

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dd7b0b2
fix(memory): isolate stored events and search results
hackermengzhi Oct 16, 2026
fbca008
Merge branch 'main' into fix/memory-event-copy-isolation
hackermengzhi Oct 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/google/adk/memory/in_memory_memory_service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ async def add_session_to_memory(self, session: Session) -> None:
deleted_ids = self._deleted_memory_ids.get(user_key, set())
self._session_events[user_key] = self._session_events.get(user_key, {})
self._session_events[user_key][session.id] = [
event
event.model_copy(deep=True)

@gemini-code-assist gemini-code-assist Bot Oct 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The Event class explicitly overrides __deepcopy__ with a custom implementation (_model_copy.deep_copy_model) to handle its copying logic correctly. Using event.model_copy(deep=True) bypasses this custom __deepcopy__ because Pydantic's model_copy deep-copies the underlying dictionary directly rather than calling copy.deepcopy on the model instance.\n\nTo ensure the custom deep copy logic is executed, use copy.deepcopy(event) instead.\n\nNote: You will need to import copy at the top of the file.

Suggested change
event.model_copy(deep=True)
copy.deepcopy(event)

@hackermengzhi hackermengzhi Oct 16, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified against the actual Pydantic implementation: BaseModel.model_copy(deep=True) without update calls self.__deepcopy__(), so these calls do invoke Event/Session's custom copier. ADK's utils/_model_copy.py::deep_copy_model also explicitly documents support for both copy.deepcopy and model_copy(deep=True).

I ran an additional probe using that production ADK helper as a custom model copier: model_copy(deep=True) invoked the override once and editing nested copied content left the source unchanged. The existing four public-service regressions and full memory-service tests already pass (53 tests). Keeping the current calls avoids a behavior-neutral rewrite; the bypass described in these comments does not occur for these calls.

for event in session.events
if event.content
and event.content.parts
Expand Down Expand Up @@ -136,7 +136,7 @@ async def add_events_to_memory(
existing_ids = {event.id for event in existing_events}
for event in events_to_add:
if event.id not in existing_ids and event.id not in deleted_ids:
existing_events.append(event)
existing_events.append(event.model_copy(deep=True))

@gemini-code-assist gemini-code-assist Bot Oct 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Similarly to add_session_to_memory, using event.model_copy(deep=True) bypasses the custom __deepcopy__ implementation defined on the Event class. Use copy.deepcopy(event) to ensure the custom copy logic is respected.

Suggested change
existing_events.append(event.model_copy(deep=True))
existing_events.append(copy.deepcopy(event))

existing_ids.add(event.id)
self._session_events[user_key][scoped_session_id] = existing_events

Expand Down Expand Up @@ -182,7 +182,7 @@ async def search_memory(
matched_words,
MemoryEntry(
id=event.id,
content=event.content,
content=event.content.model_copy(deep=True),
author=event.author,
timestamp=_utils.format_timestamp(event.timestamp),
),
Expand Down
70 changes: 70 additions & 0 deletions tests/unittests/memory/test_in_memory_memory_service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -941,3 +941,73 @@ def reader():
assert (
not errors
), f'search_memory raced with concurrent deletes: {errors[0]!r}'


@pytest.mark.parametrize('ingest_session', [True, False])
async def test_ingested_events_are_detached_from_the_caller(
ingest_session: bool,
) -> None:
"""Changing an ingested event must not rewrite stored memory."""
service = InMemoryMemoryService()
session = MOCK_SESSION_1.model_copy(deep=True)

@gemini-code-assist gemini-code-assist Bot Oct 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The Session class overrides __deepcopy__ with _model_copy.deep_copy_model. Using model_copy(deep=True) bypasses this custom copy logic. It is safer to use copy.deepcopy(MOCK_SESSION_1) instead.\n\nNote: You will need to import copy in this test file.

Suggested change
session = MOCK_SESSION_1.model_copy(deep=True)
session = copy.deepcopy(MOCK_SESSION_1)

event = session.events[0]
original_content = event.content.model_copy(deep=True)
if ingest_session:
await service.add_session_to_memory(session)
else:
await service.add_events_to_memory(
app_name=session.app_name,
user_id=session.user_id,
session_id=session.id,
events=[event],
)

event.id = 'changed-id'
event.content.parts[0].text = 'replaced by the caller'
result = await service.search_memory(
app_name=session.app_name, user_id=session.user_id, query='toolkit'
)

assert len(result.memories) == 1
assert result.memories[0].id == 'event-1a'
assert result.memories[0].content == original_content


@pytest.mark.parametrize('ingest_session', [True, False])
async def test_search_results_are_detached_from_stored_memory(
ingest_session: bool,
) -> None:
"""Editing returned content must not affect memory or the source session."""
service = InMemoryMemoryService()
session = MOCK_SESSION_1.model_copy(deep=True)

@gemini-code-assist gemini-code-assist Bot Oct 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Similarly, use copy.deepcopy(MOCK_SESSION_1) to respect the custom __deepcopy__ implementation of the Session class.

Suggested change
session = MOCK_SESSION_1.model_copy(deep=True)
session = copy.deepcopy(MOCK_SESSION_1)

event = session.events[0]
event.content.parts.append(
types.Part.from_function_call(
name='lookup', args={'options': {'color': 'blue'}}
)
)
original_content = event.content.model_copy(deep=True)
if ingest_session:
await service.add_session_to_memory(session)
else:
await service.add_events_to_memory(
app_name=session.app_name,
user_id=session.user_id,
session_id=session.id,
events=[event],
)
result = await service.search_memory(
app_name=session.app_name, user_id=session.user_id, query='toolkit'
)

returned_content = result.memories[0].content
returned_content.parts[0].text = 'replaced by the caller'
returned_content.parts[1].function_call.args['options']['color'] = 'red'
returned_content.parts.append(types.Part(text='extra content'))
repeated = await service.search_memory(
app_name=session.app_name, user_id=session.user_id, query='toolkit'
)

assert len(repeated.memories) == 1
assert repeated.memories[0].content == original_content
assert event.content == original_content
Loading