From 8d4eb109881a1379292f8b94a8adbb846c6a6fe4 Mon Sep 17 00:00:00 2001 From: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com> Date: Mon, 26 Oct 2026 23:18:33 +0530 Subject: [PATCH] Python: give memory topic files an injective filename stem Distinct topics can share a readable slug, so a slug-only stem let the second topic silently overwrite the first. The stem now appends a digest of the normalized topic, and the storage layer keeps one authoritative record: lookups validate every candidate against the record it actually holds instead of accepting a filename prefix, so a missing topic can never read, merge into, or delete a longer slug; rewriting a topic that lived at a legacy slug-only file retires the legacy file; index pointer links and provider lookups use the persisted stem; and an explicit slug override is persisted in the record and stays findable by that slug, without letting lookups for other names bleed into the overridden record. Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com> --- .../core/agent_framework/_harness/_memory.py | 120 ++++++++++++-- .../core/tests/core/test_harness_memory.py | 154 ++++++++++++++++++ 2 files changed, 264 insertions(+), 10 deletions(-) diff --git a/python/packages/core/agent_framework/_harness/_memory.py b/python/packages/core/agent_framework/_harness/_memory.py index c3f02f883b5..ea6ad95f244 100644 --- a/python/packages/core/agent_framework/_harness/_memory.py +++ b/python/packages/core/agent_framework/_harness/_memory.py @@ -3,6 +3,7 @@ from __future__ import annotations import asyncio +import hashlib import json import logging import re @@ -129,6 +130,22 @@ def _slugify_topic(topic: str) -> str: return slug or "memory-topic" +def _topic_stem(topic: str, *, slug_override: str | None = None) -> str: + """Return the injective filename stem for a topic. + + Distinct topics can share a readable slug: ``C ++`` and ``C #`` both slugify + to ``c``, so a slug-only stem let the second topic silently overwrite the + first topic's memories. The stem keeps the readable slug for humans and + appends a short digest of the normalized topic, so distinct normalized + topics never share a file (mirroring how ``_storage_key_segment`` keeps + byte-distinct identifiers in separate storage locations). + """ + normalized = _normalize_topic(topic) + slug = _slugify_topic(slug_override if slug_override is not None else normalized) + digest = hashlib.sha256(normalized.encode("utf-8")).hexdigest()[:8] + return f"{slug}-{digest}" + + def _timestamp(value: datetime | None = None) -> str: current = value or datetime.now(timezone.utc) return current.replace(microsecond=0).isoformat() @@ -302,7 +319,9 @@ def from_topic_record(cls, record: MemoryTopicRecord) -> MemoryIndexEntry: """ return cls( topic=record.topic, - slug=record.slug, + # The persisted stem, not the readable slug: distinct topics can + # share a slug, and an index link must point at the real file. + slug=record.storage_stem, summary=_coerce_summary(record.summary, record.memories), updated_at=record.updated_at, ) @@ -373,6 +392,11 @@ def __init__( self.updated_at = updated_at self.session_ids = _dedupe_strings(session_ids or []) + @property + def storage_stem(self) -> str: + """Filename stem this record persists under (slug or explicit override plus digest).""" + return _topic_stem(self.topic, slug_override=self.slug) + def to_dict(self) -> dict[str, Any]: """Serialize the topic record for tool output. @@ -449,9 +473,13 @@ def to_markdown(self) -> str: memory_lines = [f"- {_escape_markdown_line(memory)}" for memory in self.memories] or [ f"- {DEFAULT_MEMORY_NO_TOPICS_TEXT[2:]}" ] + header_lines = [f"# {self.topic}", ""] + if self.slug != _slugify_topic(self.topic): + # Persist an explicit slug override so later lookups by that slug, + # and the index stem, still resolve after a reload. + header_lines += [f"Slug: {self.slug}", ""] return "\n".join([ - f"# {self.topic}", - "", + *header_lines, f"Updated: {self.updated_at}", f"Sessions: {session_line}", "", @@ -479,6 +507,7 @@ def from_markdown(cls, markdown: str, *, fallback_topic: str | None = None) -> M ValueError: If the markdown cannot be parsed into the expected format. """ topic = fallback_topic + slug: str | None = None updated_at = _timestamp() session_ids: list[str] = [] summary_lines: list[str] = [] @@ -492,6 +521,9 @@ def from_markdown(cls, markdown: str, *, fallback_topic: str | None = None) -> M topic = stripped[2:].strip() current_section = None continue + if current_section is None and stripped.startswith("Slug: "): + slug = stripped.removeprefix("Slug: ").strip() + continue if current_section is None and stripped.startswith("Updated: "): updated_at = stripped.removeprefix("Updated: ").strip() continue @@ -522,6 +554,7 @@ def from_markdown(cls, markdown: str, *, fallback_topic: str | None = None) -> M raise ValueError("Memory topic markdown is missing a '# ' heading.") return cls( topic=topic, + slug=slug, summary="\n".join(summary_lines).strip(), memories=memories, updated_at=updated_at, @@ -763,9 +796,67 @@ def _get_index_path(self, session: AgentSession, *, source_id: str) -> Path: def _get_state_path(self, session: AgentSession, *, source_id: str) -> Path: return self._get_memory_root(session, source_id=source_id) / self.state_file_name - def _topic_path(self, session: AgentSession, *, source_id: str, topic: str) -> Path: + def _topic_path( + self, session: AgentSession, *, source_id: str, topic: str, slug_override: str | None = None + ) -> Path: + return ( + self._get_topics_directory(session, source_id=source_id) + / f"{_topic_stem(topic, slug_override=slug_override)}.md" + ) + + def _legacy_topic_path(self, session: AgentSession, *, source_id: str, topic: str) -> Path: + """Filename stem used before the digest suffix (kept for reading old stores).""" return self._get_topics_directory(session, source_id=source_id) / f"{_slugify_topic(topic)}.md" + def _existing_topic_path(self, session: AgentSession, *, source_id: str, topic: str) -> Path | None: + """Return the topic file to read, or ``None`` when no file matches. + + Lookups resolve in order: the digest stem of the given name (the exact + file ``write_topic`` uses for that topic), a legacy slug-only file from + stores written before the digest suffix, then any file whose stem starts + with the readable slug followed by a dash, which serves lookups made by + slug or by a topic that normalizes differently than its stored spelling. + When several topics share a slug, a slug-only lookup is ambiguous by + nature; the file whose recorded topic matches the lookup wins, else the + first stem in sorted order keeps the choice deterministic. + """ + topics_dir = self._get_topics_directory(session, source_id=source_id) + if not topics_dir.is_dir(): + return None + try: + wanted = _normalize_topic(topic) + except ValueError: + return None + # 1. The digest stem of the given name: the exact file write_topic uses. + topic_path = self._topic_path(session, source_id=source_id, topic=topic) + if topic_path.exists(): + return topic_path + # 2. An exact filename match: a legacy slug-only file from stores written + # before the digest suffix, or a caller passing a persisted stem. + exact_path = topics_dir / f"{_slugify_topic(topic)}.md" + if exact_path.exists(): + return exact_path + # 3. Digest-suffixed stems sharing the slug. A prefix glob alone proves + # nothing (a missing topic must never read, merge into, or delete a + # longer unrelated slug), so every candidate is validated against the + # record it actually holds. + prefix = f"{_slugify_topic(topic)}-" + matches = sorted(topics_dir.glob(f"{prefix}*.md")) + if not matches: + return None + for path in matches: + record = MemoryTopicRecord.from_markdown(path.read_text(encoding="utf-8"), fallback_topic=topic) + if record.topic == wanted: + return path + # A record carrying an explicit slug override stays findable by that + # slug; a default record never matches here, so colliding topics such + # as C++ and C# cannot merge through a shared slug. + for path in matches: + record = MemoryTopicRecord.from_markdown(path.read_text(encoding="utf-8"), fallback_topic=topic) + if record.slug == _slugify_topic(topic) and record.slug != _slugify_topic(record.topic): + return path + return None + @staticmethod def _serialize_json(value: object, *, dumps: JsonDumps) -> str: serialized = dumps(value) @@ -818,20 +909,29 @@ def list_topics(self, session: AgentSession, *, source_id: str) -> list[MemoryTo def get_topic(self, session: AgentSession, *, source_id: str, topic: str) -> MemoryTopicRecord: """Return one topic memory file by topic name or slug.""" - topic_path = self._topic_path(session, source_id=source_id, topic=topic) - if not topic_path.exists(): + topic_path = self._existing_topic_path(session, source_id=source_id, topic=topic) + if topic_path is None: raise FileNotFoundError(f"No memory topic named '{topic}' was found for this owner.") return MemoryTopicRecord.from_markdown(topic_path.read_text(encoding="utf-8"), fallback_topic=topic) def write_topic(self, session: AgentSession, record: MemoryTopicRecord, *, source_id: str) -> None: - """Persist one topic memory file.""" - topic_path = self._topic_path(session, source_id=source_id, topic=record.slug) + """Persist one topic memory file. + + The digest-stemmed file is the single authoritative record: when this + topic previously lived at another path (a legacy slug-only file, or a + stem written under a different slug override), that file is retired so + lookups can never surface stale content or resurrect the old record. + """ + previous_path = self._existing_topic_path(session, source_id=source_id, topic=record.topic) + topic_path = self._topic_path(session, source_id=source_id, topic=record.topic, slug_override=record.slug) _atomic_write_text(topic_path, f"{record.to_markdown()}\n") + if previous_path is not None and previous_path != topic_path: + previous_path.unlink(missing_ok=True) def delete_topic(self, session: AgentSession, *, source_id: str, topic: str) -> None: """Delete one topic memory file.""" - topic_path = self._topic_path(session, source_id=source_id, topic=topic) - if not topic_path.exists(): + topic_path = self._existing_topic_path(session, source_id=source_id, topic=topic) + if topic_path is None: raise FileNotFoundError(f"No memory topic named '{topic}' was found for this owner.") topic_path.unlink() diff --git a/python/packages/core/tests/core/test_harness_memory.py b/python/packages/core/tests/core/test_harness_memory.py index 464255d0b43..4e034be2de3 100644 --- a/python/packages/core/tests/core/test_harness_memory.py +++ b/python/packages/core/tests/core/test_harness_memory.py @@ -1046,6 +1046,160 @@ def test_memory_file_store_encodes_non_ascii_owner_ids(tmp_path) -> None: assert roots[0] != roots[1] +def test_memory_file_store_topics_with_the_same_slug_do_not_collide(tmp_path) -> None: + """Distinct topics that share a readable slug must keep separate memory files. + + ``C ++`` and ``C #`` both slugify to ``c``, so a slug-only filename let the + second topic's write destroy the first topic's memories. + """ + store = MemoryFileStore(tmp_path, owner_state_key="owner_id") + session = AgentSession(session_id="session-1") + session.state["owner_id"] = "alice" + + first = MemoryTopicRecord( + topic="C++", summary="cpp", memories=["smart pointers"], updated_at="2026-10-26T00:00:00+00:00" + ) + second = MemoryTopicRecord( + topic="C#", summary="csharp", memories=["primary constructors"], updated_at="2026-10-26T00:00:00+00:00" + ) + store.write_topic(session, first, source_id=DEFAULT_MEMORY_SOURCE_ID) + store.write_topic(session, second, source_id=DEFAULT_MEMORY_SOURCE_ID) + + assert store.get_topic(session, source_id=DEFAULT_MEMORY_SOURCE_ID, topic="C++").memories == ["smart pointers"] + assert store.get_topic(session, source_id=DEFAULT_MEMORY_SOURCE_ID, topic="C#").memories == ["primary constructors"] + listed = sorted(entry.topic for entry in store.list_topics(session, source_id=DEFAULT_MEMORY_SOURCE_ID)) + assert listed == ["C#", "C++"] + + +def test_memory_file_store_reads_topics_from_the_legacy_slug_filename(tmp_path) -> None: + """Topic files written before the digest suffix stay readable.""" + store = MemoryFileStore(tmp_path, owner_state_key="owner_id") + session = AgentSession(session_id="session-1") + session.state["owner_id"] = "alice" + + topics_dir = store._get_topics_directory(session, source_id=DEFAULT_MEMORY_SOURCE_ID) + topics_dir.mkdir(parents=True, exist_ok=True) + (topics_dir / "python.md").write_text("# python\n\n## Memories\n\n- old note\n", encoding="utf-8") + + record = store.get_topic(session, source_id=DEFAULT_MEMORY_SOURCE_ID, topic="Python") + assert record.memories == ["old note"] + + +def test_memory_lookup_never_bleeds_across_slug_prefixes(tmp_path) -> None: + """A missing short topic must not read, merge into, or delete a longer + topic whose slug starts with the same characters.""" + store = MemoryFileStore(tmp_path, owner_state_key="owner_id") + session = AgentSession(session_id="session-1") + session.state["owner_id"] = "alice" + + store.write_topic( + session, + MemoryTopicRecord(topic="Collection", slug=None, summary="a", memories=["x"], updated_at="t"), + source_id="src", + ) + + with pytest.raises(FileNotFoundError): + store.get_topic(session, source_id="src", topic="coll") + + +def test_memory_lookup_validates_identity_for_colliding_slugs(tmp_path) -> None: + """Colliding topics such as C++ and C# resolve to their own records, and a + lookup for a topic that does not exist under a shared slug finds nothing + rather than an arbitrary same-slug file.""" + store = MemoryFileStore(tmp_path, owner_state_key="owner_id") + session = AgentSession(session_id="session-1") + session.state["owner_id"] = "alice" + + store.write_topic( + session, + MemoryTopicRecord(topic="C++", slug=None, summary="cpp", memories=["smart pointers"], updated_at="t"), + source_id="src", + ) + store.write_topic( + session, + MemoryTopicRecord(topic="C#", slug=None, summary="cs", memories=["ctors"], updated_at="t"), + source_id="src", + ) + + assert store.get_topic(session, source_id="src", topic="C++").memories == ["smart pointers"] + assert store.get_topic(session, source_id="src", topic="C#").memories == ["ctors"] + with pytest.raises(FileNotFoundError): + store.get_topic(session, source_id="src", topic="C") + + +def test_memory_write_retires_a_legacy_slug_file_for_the_same_topic(tmp_path) -> None: + """Rewriting a topic that lived at a legacy slug-only filename leaves a + single authoritative record: the digest-stemmed file.""" + store = MemoryFileStore(tmp_path, owner_state_key="owner_id") + session = AgentSession(session_id="session-1") + session.state["owner_id"] = "alice" + topics_dir = store._get_topics_directory(session, source_id="src") + + legacy = topics_dir / "legacy-topic.md" + legacy.parent.mkdir(parents=True, exist_ok=True) + legacy.write_text( + "# Legacy Topic\n\nUpdated: t0\nSessions: -\n\n## Summary\nold\n\n## Memories\n- old fact\n", + encoding="utf-8", + ) + + store.write_topic( + session, + MemoryTopicRecord(topic="Legacy Topic", slug=None, summary="new", memories=["new fact"], updated_at="t1"), + source_id="src", + ) + + assert not legacy.exists() + record = store.get_topic(session, source_id="src", topic="Legacy Topic") + assert record.memories == ["new fact"] + assert sorted(path.name for path in topics_dir.glob("legacy*.md")) == [f"{record.storage_stem}.md"] + + +def test_memory_index_pointer_links_resolve_to_the_persisted_files(tmp_path) -> None: + """Index entries link the persisted stem, so every pointer line names a + file that exists even when several topics share a readable slug.""" + store = MemoryFileStore(tmp_path, owner_state_key="owner_id") + session = AgentSession(session_id="session-1") + session.state["owner_id"] = "alice" + store.write_topic( + session, + MemoryTopicRecord(topic="C++", slug=None, summary="cpp", memories=["m"], updated_at="t"), + source_id="src", + ) + store.write_topic( + session, MemoryTopicRecord(topic="C#", slug=None, summary="cs", memories=["m"], updated_at="t"), source_id="src" + ) + topics_dir = store._get_topics_directory(session, source_id="src") + + entries = store.rebuild_index(session, source_id="src", line_limit=50, line_length=200) + + assert len(entries) == 2 + assert len({entry.slug for entry in entries}) == 2 + for entry in entries: + assert f"(topics/{entry.slug}.md)" in entry.to_pointer_line() + assert (topics_dir / f"{entry.slug}.md").exists() + + +def test_memory_explicit_slug_override_is_persisted_and_findable(tmp_path) -> None: + """A record with an explicit slug override persists under that slug and is + findable by it after a reload, without letting lookups for other names + bleed into the overridden record.""" + store = MemoryFileStore(tmp_path, owner_state_key="owner_id") + session = AgentSession(session_id="session-1") + session.state["owner_id"] = "alice" + + store.write_topic( + session, + MemoryTopicRecord(topic="Machine Learning", slug="ml-notes", summary="s", memories=["fact"], updated_at="t"), + source_id="src", + ) + + record = store.get_topic(session, source_id="src", topic="ml-notes") + assert record.topic == "Machine Learning" + assert record.memories == ["fact"] + with pytest.raises(FileNotFoundError): + store.get_topic(session, source_id="src", topic="machine") + + def test_memory_file_store_uses_literal_folders_for_safe_identifiers(tmp_path) -> None: """Safe identifiers are readable on disk rather than opaque encoded segments.""" store = MemoryFileStore(tmp_path, owner_prefix="user_", owner_state_key="owner_id")