diff --git a/python/packages/core/agent_framework/_harness/_file_memory.py b/python/packages/core/agent_framework/_harness/_file_memory.py index b64a112376b..fc8dbfef189 100644 --- a/python/packages/core/agent_framework/_harness/_file_memory.py +++ b/python/packages/core/agent_framework/_harness/_file_memory.py @@ -95,9 +95,16 @@ def _description_file_name(file_name: str) -> str: """Return the companion description file name for ``file_name``. - The suffix replaces the original extension when present (so ``notes.md`` - becomes ``notes_description.md``); otherwise it is appended. + The suffix is appended to the full name. Stripping the extension first let + ``notes.md`` and ``notes`` share one sidecar, so one file's description + overwrote or deleted the other's; appending keeps the mapping injective + while extension-less names keep their existing sidecar name. """ + return f"{file_name}{_DESCRIPTION_SUFFIX}" + + +def _legacy_description_file_name(file_name: str) -> str: + """Return the sidecar name used before the suffix was made injective.""" dot_index = file_name.rfind(".") if dot_index > 0: return f"{file_name[:dot_index]}{_DESCRIPTION_SUFFIX}" @@ -323,6 +330,13 @@ def _resolve_working_folder(self, context: SessionContext) -> str: ) return _storage_key_segment(raw_scope, encoded_prefix=_ENCODED_SCOPE_PREFIX) + async def _read_description(self, working_folder: str, file_name: str) -> str | None: + """Read a file's description sidecar, including ones written under the legacy name.""" + description = await self.store.read(_combine_paths(working_folder, _description_file_name(file_name))) + if description is not None: + return description + return await self.store.read(_combine_paths(working_folder, _legacy_description_file_name(file_name))) + async def _rebuild_index(self, working_folder: str) -> None: """Rebuild the ``memories.md`` index for ``working_folder``. @@ -335,7 +349,7 @@ async def _rebuild_index(self, working_folder: str) -> None: lines = ["# Memory Index", ""] for file_name in sorted_files[:_MAX_INDEX_ENTRIES]: - description = await self.store.read(_combine_paths(working_folder, _description_file_name(file_name))) + description = await self._read_description(working_folder, file_name) if description and description.strip(): lines.append(f"- **{file_name}**: {description.strip()}") else: @@ -378,6 +392,7 @@ async def file_memory_write(file_name: str, content: str, description: str | Non path = _combine_paths(working_folder, normalized) desc_path = _combine_paths(working_folder, _description_file_name(normalized)) + legacy_desc_path = _combine_paths(working_folder, _legacy_description_file_name(normalized)) # Every mutating tool locks the whole working folder, not just the file, because each # write and delete rebuilds the folder's memories.md index. The lock is shared by all # providers on the store, including file-access tools editing a file in this folder, so @@ -387,8 +402,12 @@ async def file_memory_write(file_name: str, content: str, description: str | Non await self.store.write(path, content) if description and description.strip(): await self.store.write(desc_path, description) + if legacy_desc_path != desc_path: + await self.store.delete(legacy_desc_path) else: await self.store.delete(desc_path) + if legacy_desc_path != desc_path: + await self.store.delete(legacy_desc_path) await self._rebuild_index(working_folder) except ValueError as exc: return f"Could not write file '{file_name}': {exc}" @@ -427,10 +446,13 @@ async def file_memory_delete(file_name: str) -> str: path = _combine_paths(working_folder, normalized) desc_path = _combine_paths(working_folder, _description_file_name(normalized)) + legacy_desc_path = _combine_paths(working_folder, _legacy_description_file_name(normalized)) async with _store_write_lock(self.store, working_folder): try: deleted = await self.store.delete(path) await self.store.delete(desc_path) + if legacy_desc_path != desc_path: + await self.store.delete(legacy_desc_path) await self._rebuild_index(working_folder) except ValueError as exc: return f"Could not delete file '{file_name}': {exc}" diff --git a/python/packages/core/tests/core/test_harness_file_memory.py b/python/packages/core/tests/core/test_harness_file_memory.py index 5d8f7394be2..32570d82500 100644 --- a/python/packages/core/tests/core/test_harness_file_memory.py +++ b/python/packages/core/tests/core/test_harness_file_memory.py @@ -55,13 +55,23 @@ async def _prepare( return context, tools -def test_description_file_name_replaces_extension() -> None: - """The description sidecar replaces a known extension and appends otherwise.""" - assert _description_file_name("notes.md") == "notes_description.md" - assert _description_file_name("data.json") == "data_description.md" +def test_description_file_name_is_injective_across_extensions() -> None: + """The description sidecar appends to the full name so extensions cannot collide.""" + assert _description_file_name("notes.md") == "notes.md_description.md" + assert _description_file_name("data.json") == "data.json_description.md" assert _description_file_name("noext") == "noext_description.md" - # Leading-dot files have no stem, so the suffix is appended. + # Leading-dot files keep the suffix appended as well. assert _description_file_name(".hidden") == ".hidden_description.md" + # names with and without an extension no longer share one sidecar. + assert _description_file_name("notes.md") != _description_file_name("notes") + + +def test_legacy_description_file_name_still_resolves_for_old_stores() -> None: + """Sidecars written before the injective naming stay readable for extension files.""" + from agent_framework._harness._file_memory import _legacy_description_file_name # pyright: ignore[reportPrivateUsage] + + assert _legacy_description_file_name("notes.md") == "notes_description.md" + assert _legacy_description_file_name("noext") == "noext_description.md" def test_is_internal_file_detects_sidecars_and_index() -> None: @@ -152,7 +162,7 @@ async def test_description_sidecar_is_written_and_listed() -> None: ) assert "with description" in _text(result) - sidecar = await store.read(_combine_paths("user-1", "arch_description.md")) + sidecar = await store.read(_combine_paths("user-1", "arch.md_description.md")) assert sidecar == "system architecture" listed = json.loads(_text(await list_files.invoke())) @@ -160,7 +170,7 @@ async def test_description_sidecar_is_written_and_listed() -> None: # Re-saving without a description removes the sidecar. await save.invoke(arguments={"file_name": "arch.md", "content": "big content"}) - assert await store.read(_combine_paths("user-1", "arch_description.md")) is None + assert await store.read(_combine_paths("user-1", "arch.md_description.md")) is None listed_again = json.loads(_text(await list_files.invoke())) assert listed_again == [{"name": "arch.md", "type": "file", "description": None}] @@ -172,10 +182,53 @@ async def test_delete_removes_sidecar() -> None: _, tools = await _prepare(provider) await tools["file_memory_write"].invoke(arguments={"file_name": "arch.md", "content": "x", "description": "desc"}) - assert await store.read(_combine_paths("user-1", "arch_description.md")) == "desc" + assert await store.read(_combine_paths("user-1", "arch.md_description.md")) == "desc" await tools["file_memory_delete"].invoke(arguments={"file_name": "arch.md"}) - assert await store.read(_combine_paths("user-1", "arch_description.md")) is None + assert await store.read(_combine_paths("user-1", "arch.md_description.md")) is None + + +async def test_description_sidecars_stay_separate_for_names_differing_by_extension() -> None: + """``notes.md`` and ``notes`` own distinct description sidecars.""" + store = InMemoryAgentFileStore() + provider = FileMemoryProvider(store=store, scope="user-1") + _, tools = await _prepare(provider) + save = tools["file_memory_write"] + list_files = tools["file_memory_ls"] + + await save.invoke( + arguments={"file_name": "notes.md", "content": "architecture notes", "description": "architecture"} + ) + await save.invoke(arguments={"file_name": "notes", "content": "scratch", "description": "scratch"}) + + assert await store.read(_combine_paths("user-1", "notes.md_description.md")) == "architecture" + assert await store.read(_combine_paths("user-1", "notes_description.md")) == "scratch" + + # Rewriting the extension-less file without a description must not touch the other's sidecar. + await save.invoke(arguments={"file_name": "notes", "content": "more scratch"}) + assert await store.read(_combine_paths("user-1", "notes.md_description.md")) == "architecture" + + listed = json.loads(_text(await list_files.invoke())) + descriptions = {entry["name"]: entry["description"] for entry in listed} + assert descriptions == {"notes.md": "architecture", "notes": None} + + +async def test_legacy_description_sidecar_still_feeds_the_index() -> None: + """Descriptions written under the old sidecar name keep appearing in the index.""" + store = InMemoryAgentFileStore() + provider = FileMemoryProvider(store=store, scope="user-1") + _, tools = await _prepare(provider) + + await store.write(_combine_paths("user-1", "legacy.md"), "old") + await store.write(_combine_paths("user-1", "legacy_description.md"), "old description") + await tools["file_memory_write"].invoke( + arguments={"file_name": "fresh.txt", "content": "new", "description": "new desc"} + ) + + index = await store.read(_combine_paths("user-1", _MEMORY_INDEX_FILE_NAME)) + assert index is not None + assert "old description" in index + assert "new desc" in index async def test_index_is_rebuilt_and_injected_on_next_run() -> None: