From a213f7eafd6d8f664b930bc0cf158e0795dafd55 Mon Sep 17 00:00:00 2001 From: Lushiyao5 <321342421+Lushiyao5@users.noreply.github.com> Date: Sat, 31 Oct 2026 17:21:50 -0500 Subject: [PATCH 1/4] Python: fix file memory description sidecar collisions The description sidecar name replaced the memory file's extension, so `notes.md` and `notes` both mapped to `notes_description.md` and the two memories overwrote (and deleted) each other's description. Append the suffix to the whole memory file name instead, which keeps the mapping injective while leaving the internal-file hiding rules unchanged because every sidecar still ends in `_description.md`. Fixes #9871 --- python/packages/core/AGENTS.md | 2 +- .../agent_framework/_harness/_file_memory.py | 15 +-- .../tests/core/test_harness_file_memory.py | 101 ++++++++++++++++-- 3 files changed, 101 insertions(+), 17 deletions(-) diff --git a/python/packages/core/AGENTS.md b/python/packages/core/AGENTS.md index 3acfc513d51..f47e4074fd7 100644 --- a/python/packages/core/AGENTS.md +++ b/python/packages/core/AGENTS.md @@ -290,7 +290,7 @@ The vector store API is experimental under the shared `VECTOR_STORES` feature ID - **`FileMemoryProvider`** - `ContextProvider` that gives an agent a session-scoped, file-based memory backed by the same `AgentFileStore` abstraction. Adds tools (`file_memory_write`, `file_memory_read`, `file_memory_delete`, `file_memory_ls`, `file_memory_grep`, `file_memory_replace`, `file_memory_replace_lines`) plus default usage instructions. Port of the .NET `FileMemoryProvider`. - **Scoping** - Memories are isolated per session by default: each session writes under a working folder derived from `context.session_id`. Pass an explicit `scope` (e.g. a user id) to group memories across sessions, mirroring `FoundryMemoryProvider`'s `scope` arg. The scope (or session id) is an **opaque namespace key, not a path**: it is mapped to exactly one folder via the shared `agent_framework._filesystem._storage_key_segment` derivation, which is **injective** (except for a collision-resistant digest fallback past a length cap) — two byte-distinct values do not share a working folder, so a caller authorized for one identifier cannot reach another's memories. A multi-segment value such as `"tenants/alice"` therefore becomes a single encoded folder rather than a nested directory. The provider **fails closed** (`ValueError`) when neither a `scope` nor a `session_id` is available, because an empty working folder would be the shared store root. Applications should still canonicalize and authorize externally supplied identifiers themselves; the storage mapping is the storage-layer guarantee, not a substitute for that check. -- **Descriptions & index** - `file_memory_write` accepts an optional `description`, stored in a companion `_description.md` sidecar. After each write/delete the provider rebuilds a capped (50-entry) `memories.md` index, and `before_run` injects that index as a `user` context message so the model knows what memories exist. Sidecars and the index are internal files hidden from `file_memory_ls`/`file_memory_grep` and rejected as write targets. +- **Descriptions & index** - `file_memory_write` accepts an optional `description`, stored in a companion sidecar named `_description.md` (the whole memory file name is kept, so `notes.md` and `notes` get distinct sidecars). After each write/delete the provider rebuilds a capped (50-entry) `memories.md` index, and `before_run` injects that index as a `user` context message so the model knows what memories exist. Sidecars and the index are internal files hidden from `file_memory_ls`/`file_memory_grep` and rejected as write targets. Sidecars written by earlier versions used the extension-less stem (`notes_description.md`) and are not migrated. - **`DEFAULT_FILE_MEMORY_SOURCE_ID`** / **`DEFAULT_FILE_MEMORY_INSTRUCTIONS`** - Public defaults for the provider's source id and instruction banner. - **Harness wiring** - `create_harness_agent` includes the `FileMemoryProvider` by default; the `FileAccessProvider` is opt-in and added only when a `file_access_store` is supplied (no implicit `{cwd}/working` store is created). Disable file memory via `disable_file_memory`; override its backing store via `file_memory_store`. When no file-memory store is supplied, the default is `FileSystemAgentFileStore` rooted at `{cwd}/agent-file-memory`. `create_harness_agent` also wires in `MessageInjectionMiddleware` by default (mirroring the .NET harness's `UseMessageInjection`); it is always on with no opt-out because it is a no-op when no messages are queued for the session. - **Experimental-feature gating** - `create_harness_agent` itself is released (no longer `@experimental`), but a few features it can wire in remain experimental or pre-release: **background agents** (`background_agents`), **file access** (`file_access_store`), **looping** (`loop_should_continue`), and the **shell tooling** (`shell_executor`, from the pre-release `agent-framework-tools` package). Enabling any of them emits a single `ExperimentalWarning` (naming the responsible parameter). The three experimental harness providers share one `HARNESS` dedup key so the downstream experimental provider does not warn a second time; the shell tooling uses a separate dedup key (it is not a `HARNESS`-decorated feature). diff --git a/python/packages/core/agent_framework/_harness/_file_memory.py b/python/packages/core/agent_framework/_harness/_file_memory.py index b64a112376b..543e08af3bc 100644 --- a/python/packages/core/agent_framework/_harness/_file_memory.py +++ b/python/packages/core/agent_framework/_harness/_file_memory.py @@ -4,8 +4,9 @@ :class:`FileMemoryProvider` gives an agent a session-scoped, file-based memory system. Each memory is stored as an individual file with a meaningful name, and -large files can carry a companion description file (suffixed with -``_description.md``) that provides a short summary used for discovery. A +large files can carry a companion description file (the memory file name with +``_description.md`` appended, so the sidecar of ``notes.md`` is +``notes.md_description.md``) that provides a short summary used for discovery. A ``memories.md`` index file is maintained automatically and injected into the agent's context so the model knows what memories already exist. @@ -95,12 +96,12 @@ 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 *whole* memory file name, so ``notes.md`` + becomes ``notes.md_description.md`` while ``notes`` becomes + ``notes_description.md``. Keeping the extension in the derivation makes + the mapping injective: distinct memory files always get distinct sidecars, + so writes and deletes can never clobber another memory's description. """ - dot_index = file_name.rfind(".") - if dot_index > 0: - return f"{file_name[:dot_index]}{_DESCRIPTION_SUFFIX}" return f"{file_name}{_DESCRIPTION_SUFFIX}" 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..1c3e1454a84 100644 --- a/python/packages/core/tests/core/test_harness_file_memory.py +++ b/python/packages/core/tests/core/test_harness_file_memory.py @@ -55,12 +55,14 @@ 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_appends_to_full_name() -> None: + """The description sidecar appends to the whole file name, so it is injective.""" + assert _description_file_name("notes.md") == "notes.md_description.md" + assert _description_file_name("notes") == "notes_description.md" + # Names that share a stem must not share a sidecar. + assert _description_file_name("notes.md") != _description_file_name("notes") + 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. assert _description_file_name(".hidden") == ".hidden_description.md" @@ -152,7 +154,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 +162,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 +174,91 @@ 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_sidecars_of_names_sharing_a_stem_do_not_collide() -> None: + """``notes.md`` and ``notes`` must not share one description sidecar.""" + store = InMemoryAgentFileStore() + provider = FileMemoryProvider(store=store, scope="user-1") + _, tools = await _prepare(provider) + save = tools["file_memory_write"] + + await save.invoke(arguments={"file_name": "notes.md", "content": "md body", "description": "markdown notes"}) + await save.invoke(arguments={"file_name": "notes", "content": "plain body", "description": "plain notes"}) + + # Each memory file keeps its own sidecar, and the second write did not + # overwrite the first one's description. + assert await store.read(_combine_paths("user-1", "notes.md_description.md")) == "markdown notes" + assert await store.read(_combine_paths("user-1", "notes_description.md")) == "plain notes" + assert sorted(entry.name for entry in await store.list_children("user-1")) == [ + "memories.md", + "notes", + "notes.md", + "notes.md_description.md", + "notes_description.md", + ] + + listed = json.loads(_text(await tools["file_memory_ls"].invoke())) + assert sorted(listed, key=lambda entry: entry["name"]) == [ + {"name": "notes", "type": "file", "description": "plain notes"}, + {"name": "notes.md", "type": "file", "description": "markdown notes"}, + ] + + index = await store.read(_combine_paths("user-1", _MEMORY_INDEX_FILE_NAME)) + assert index is not None + assert "- **notes.md**: markdown notes" in index + assert "- **notes**: plain notes" in index + + +async def test_rewriting_one_file_without_description_keeps_sibling_sidecar() -> None: + """Dropping a description on one file must not delete another file's sidecar.""" + store = InMemoryAgentFileStore() + provider = FileMemoryProvider(store=store, scope="user-1") + _, tools = await _prepare(provider) + save = tools["file_memory_write"] + + await save.invoke(arguments={"file_name": "notes.md", "content": "md body", "description": "markdown notes"}) + await save.invoke(arguments={"file_name": "notes", "content": "plain body", "description": "plain notes"}) + + # Rewrite ``notes.md`` without a description: only its own sidecar goes away. + await save.invoke(arguments={"file_name": "notes.md", "content": "rewritten"}) + assert await store.read(_combine_paths("user-1", "notes.md_description.md")) is None + assert await store.read(_combine_paths("user-1", "notes_description.md")) == "plain notes" + + listed = json.loads(_text(await tools["file_memory_ls"].invoke())) + assert sorted(listed, key=lambda entry: entry["name"]) == [ + {"name": "notes", "type": "file", "description": "plain notes"}, + {"name": "notes.md", "type": "file", "description": None}, + ] + + +async def test_deleting_one_file_keeps_sibling_sidecar() -> None: + """Deleting one memory must not delete another memory's description.""" + store = InMemoryAgentFileStore() + provider = FileMemoryProvider(store=store, scope="user-1") + _, tools = await _prepare(provider) + save = tools["file_memory_write"] + delete = tools["file_memory_delete"] + + await save.invoke(arguments={"file_name": "notes.md", "content": "md body", "description": "markdown notes"}) + await save.invoke(arguments={"file_name": "notes", "content": "plain body", "description": "plain notes"}) + + await delete.invoke(arguments={"file_name": "notes"}) + assert await store.read(_combine_paths("user-1", "notes_description.md")) is None + assert await store.read(_combine_paths("user-1", "notes.md_description.md")) == "markdown notes" + + listed = json.loads(_text(await tools["file_memory_ls"].invoke())) + assert listed == [{"name": "notes.md", "type": "file", "description": "markdown notes"}] + + # The sidecars stay internal: they are neither listed nor valid write targets. + assert all(not _is_internal_file(entry["name"]) for entry in listed) + for reserved in ("notes.md_description.md", "notes_description.md"): + rejected = await save.invoke(arguments={"file_name": reserved, "content": "x"}) + assert "reserved" in _text(rejected) async def test_index_is_rebuilt_and_injected_on_next_run() -> None: From 85c87ce4974b5aa2b95c495a9185639136f32f55 Mon Sep 17 00:00:00 2001 From: Lushiyao5 <321342421+Lushiyao5@users.noreply.github.com> Date: Sat, 31 Oct 2026 20:18:56 -0500 Subject: [PATCH 2/4] Python: fall back to a digest sidecar when the appended name exceeds filesystem limits Appending _description.md to a near-limit memory file name could produce a sidecar past the common 255-byte filename cap, leaving the memory body written but its description unwritable. Names that would overflow now derive a SHA-256 digest sidecar that keeps the _description.md suffix, so listings still hide it and distinct long names stay distinct. --- .../agent_framework/_harness/_file_memory.py | 20 +++++++++++++++- .../tests/core/test_harness_file_memory.py | 23 +++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/python/packages/core/agent_framework/_harness/_file_memory.py b/python/packages/core/agent_framework/_harness/_file_memory.py index 543e08af3bc..a41b3f2387d 100644 --- a/python/packages/core/agent_framework/_harness/_file_memory.py +++ b/python/packages/core/agent_framework/_harness/_file_memory.py @@ -43,6 +43,7 @@ from __future__ import annotations +import hashlib import logging from typing import Annotated, Any @@ -87,10 +88,16 @@ _DESCRIPTION_SUFFIX = "_description.md" _MEMORY_INDEX_FILE_NAME = "memories.md" _MAX_INDEX_ENTRIES = 50 +# Filesystem name limits are commonly 255 bytes; the sidecar derivation must stay +# within that budget so every valid memory file name also gets a writable sidecar. +_MAX_DESCRIPTION_FILE_NAME_BYTES = 255 # Prefix for scopes that are not usable verbatim as a folder name. Kept distinct # from the other harness prefixes so the same value under a different component # never lands in the same directory. _ENCODED_SCOPE_PREFIX = "~scope-" +# Prefix for description sidecars of memory file names too long to carry the +# suffix verbatim. Distinct from the other prefixes for the same reason. +_DIGEST_DESCRIPTION_PREFIX = "~memory-desc-" def _description_file_name(file_name: str) -> str: @@ -101,8 +108,19 @@ def _description_file_name(file_name: str) -> str: ``notes_description.md``. Keeping the extension in the derivation makes the mapping injective: distinct memory files always get distinct sidecars, so writes and deletes can never clobber another memory's description. + + A memory file name long enough to push the appended sidecar past the + filesystem's name limit cannot carry the suffix verbatim, so such names + fall back to a SHA-256 digest (still suffixed, still injective in + practice). The two namespaces cannot collide: a memory file name whose + sidecar would look like a digest sidecar is itself far shorter than the + names that take the digest branch. """ - return f"{file_name}{_DESCRIPTION_SUFFIX}" + sidecar = f"{file_name}{_DESCRIPTION_SUFFIX}" + if len(sidecar.encode("utf-8")) <= _MAX_DESCRIPTION_FILE_NAME_BYTES: + return sidecar + digest = hashlib.sha256(file_name.encode("utf-8")).hexdigest() + return f"{_DIGEST_DESCRIPTION_PREFIX}{digest}{_DESCRIPTION_SUFFIX}" def _is_internal_file(file_name: str) -> bool: 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 1c3e1454a84..fbb391702eb 100644 --- a/python/packages/core/tests/core/test_harness_file_memory.py +++ b/python/packages/core/tests/core/test_harness_file_memory.py @@ -66,6 +66,29 @@ def test_description_file_name_appends_to_full_name() -> None: assert _description_file_name(".hidden") == ".hidden_description.md" +def test_description_file_name_stays_within_filesystem_name_limits() -> None: + """Sidecars of very long memory file names must stay writable and distinct.""" + # "a"*235 + ".md" carries the suffix verbatim at exactly 254 bytes. + within_limit = "a" * 235 + ".md" + # "a"*238 + ".md" would push the appended sidecar to 256 bytes. + over_limit = "a" * 238 + ".md" + assert len(_description_file_name(within_limit).encode("utf-8")) <= 255 + assert _description_file_name(within_limit) == within_limit + "_description.md" + + bounded = _description_file_name(over_limit) + assert len(bounded.encode("utf-8")) <= 255 + # The digest sidecar keeps the suffix, so listings still hide it. + assert _is_internal_file(bounded) + + # Distinct long names must not share a digest sidecar. + other = "b" * 238 + ".md" + assert _description_file_name(over_limit) != _description_file_name(other) + # The derivation is stable, so reads and deletes resolve to the same sidecar. + assert _description_file_name(over_limit) == _description_file_name(over_limit) + # A digest sidecar never equals a verbatim sidecar of a shorter name. + assert _description_file_name(over_limit) != _description_file_name("~memory-desc-x") + + def test_is_internal_file_detects_sidecars_and_index() -> None: """Internal files are description sidecars and the memory index, case-insensitively.""" assert _is_internal_file("notes_description.md") From b3ec13567c3744e3ec32586a778112fadab4f297 Mon Sep 17 00:00:00 2001 From: Lushiyao5 <321342421+Lushiyao5@users.noreply.github.com> Date: Mon, 2 Nov 2026 06:42:32 -0500 Subject: [PATCH 3/4] Python: keep digest and verbatim description sidecars disjoint A valid short memory name beginning with the reserved digest prefix could resolve to the same sidecar as a long name with the matching digest, so writes and deletes crossed memories without any SHA-256 collision. Names starting with ~memory-desc- now always take the digest branch, which keeps the verbatim namespace free of the reserved prefix. Add the requested cross-branch lifecycle regression (write/list/read/delete/rewrite) using a long name and its digest-stem short name, plus a disk-backed regression with FileSystemAgentFileStore. Both new lifecycle tests fail on the previous derivation; the file-memory suite passes 43 tests and ruff is clean. --- .../agent_framework/_harness/_file_memory.py | 14 ++- .../tests/core/test_harness_file_memory.py | 101 ++++++++++++++++++ 2 files changed, 111 insertions(+), 4 deletions(-) diff --git a/python/packages/core/agent_framework/_harness/_file_memory.py b/python/packages/core/agent_framework/_harness/_file_memory.py index a41b3f2387d..83737d7fc58 100644 --- a/python/packages/core/agent_framework/_harness/_file_memory.py +++ b/python/packages/core/agent_framework/_harness/_file_memory.py @@ -97,6 +97,9 @@ _ENCODED_SCOPE_PREFIX = "~scope-" # Prefix for description sidecars of memory file names too long to carry the # suffix verbatim. Distinct from the other prefixes for the same reason. +# Memory file names that already start with this prefix also take the digest +# branch, so the verbatim namespace never contains the prefix and the two +# branches cannot produce the same sidecar. _DIGEST_DESCRIPTION_PREFIX = "~memory-desc-" @@ -112,12 +115,15 @@ def _description_file_name(file_name: str) -> str: A memory file name long enough to push the appended sidecar past the filesystem's name limit cannot carry the suffix verbatim, so such names fall back to a SHA-256 digest (still suffixed, still injective in - practice). The two namespaces cannot collide: a memory file name whose - sidecar would look like a digest sidecar is itself far shorter than the - names that take the digest branch. + practice). Names that already start with the reserved digest prefix take + the digest branch too: otherwise a short memory named after another name's + digest stem would share that name's sidecar without any hash collision. """ sidecar = f"{file_name}{_DESCRIPTION_SUFFIX}" - if len(sidecar.encode("utf-8")) <= _MAX_DESCRIPTION_FILE_NAME_BYTES: + if ( + not file_name.startswith(_DIGEST_DESCRIPTION_PREFIX) + and len(sidecar.encode("utf-8")) <= _MAX_DESCRIPTION_FILE_NAME_BYTES + ): return sidecar digest = hashlib.sha256(file_name.encode("utf-8")).hexdigest() return f"{_DIGEST_DESCRIPTION_PREFIX}{digest}{_DESCRIPTION_SUFFIX}" 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 fbb391702eb..9231e1ae8d9 100644 --- a/python/packages/core/tests/core/test_harness_file_memory.py +++ b/python/packages/core/tests/core/test_harness_file_memory.py @@ -14,6 +14,7 @@ Content, FileAccessProvider, FileMemoryProvider, + FileSystemAgentFileStore, FunctionTool, InMemoryAgentFileStore, ) @@ -87,6 +88,16 @@ def test_description_file_name_stays_within_filesystem_name_limits() -> None: assert _description_file_name(over_limit) == _description_file_name(over_limit) # A digest sidecar never equals a verbatim sidecar of a shorter name. assert _description_file_name(over_limit) != _description_file_name("~memory-desc-x") + # Reserved-prefix names take the digest branch even when short, so the + # verbatim namespace can never contain the digest prefix and the two + # branches are disjoint without relying on a SHA-256 collision. + import hashlib + + digest_stem = "~memory-desc-" + hashlib.sha256(over_limit.encode()).hexdigest() + assert digest_stem.startswith("~memory-desc-") + assert _description_file_name(digest_stem) != digest_stem + "_description.md" + assert _description_file_name(digest_stem) != _description_file_name(over_limit) + assert _description_file_name("~memory-desc-x") != "~memory-desc-x_description.md" def test_is_internal_file_detects_sidecars_and_index() -> None: @@ -190,6 +201,96 @@ async def test_description_sidecar_is_written_and_listed() -> None: assert listed_again == [{"name": "arch.md", "type": "file", "description": None}] +async def test_long_name_and_matching_digest_stem_short_name_have_independent_lifecycles() -> None: + """A long name and a valid short name equal to its digest stem must not share a sidecar. + + The short name begins with the reserved digest prefix, so it also takes the + digest branch; writing or deleting either memory must leave the other + memory's body and description untouched. + """ + import hashlib + + store = InMemoryAgentFileStore() + provider = FileMemoryProvider(store=store, scope="user-1") + _, tools = await _prepare(provider) + save = tools["file_memory_write"] + read = tools["file_memory_read"] + delete = tools["file_memory_delete"] + list_files = tools["file_memory_ls"] + + long_name = "a" * 238 + ".md" + short_name = "~memory-desc-" + hashlib.sha256(long_name.encode()).hexdigest() + # Both names are accepted by the provider's validation and normalization. + assert _description_file_name(long_name) != _description_file_name(short_name) + + await save.invoke(arguments={"file_name": long_name, "content": "long-body", "description": "long-description"}) + await save.invoke(arguments={"file_name": short_name, "content": "short-body", "description": "short-description"}) + + # Writing the short memory must not overwrite the long memory's description. + listed = {entry["name"]: entry["description"] for entry in json.loads(_text(await list_files.invoke()))} + assert listed[long_name] == "long-description" + assert listed[short_name] == "short-description" + assert _text(await read.invoke(arguments={"file_name": long_name})) == "long-body" + assert _text(await read.invoke(arguments={"file_name": short_name})) == "short-body" + + # Deleting the short memory must not remove the long memory's description. + await delete.invoke(arguments={"file_name": short_name}) + listed_after = {entry["name"]: entry["description"] for entry in json.loads(_text(await list_files.invoke()))} + assert listed_after == {long_name: "long-description"} + assert _text(await read.invoke(arguments={"file_name": long_name})) == "long-body" + + # Rewriting the long memory still updates only its own sidecar. + await save.invoke(arguments={"file_name": long_name, "content": "long-body-2", "description": "long-description-2"}) + listed_rewritten = {entry["name"]: entry["description"] for entry in json.loads(_text(await list_files.invoke()))} + assert listed_rewritten == {long_name: "long-description-2"} + + +async def test_long_name_sidecar_is_writable_on_disk(tmp_path, monkeypatch) -> None: + """A verbatim sidecar over the name limit must fall back to a writable digest on disk. + + Uses the real file system rather than the in-memory store: the long memory + body is written before sidecar access, so an over-limit sidecar would leave + partial state and break later index rebuilds. Windows path limits reject + the longest names, so there the test uses a shorter name and a lower + sidecar budget; Linux CI uses the real 255-byte limit. + """ + import hashlib + import sys + + import agent_framework._harness._file_memory as file_memory + + long_name = "a" * 238 + ".md" + if sys.platform == "win32": + # Windows rejects long names once the full temporary path grows past + # its path limit, so exercise the same derivation with a shorter name + # and a proportionally lower sidecar budget. Linux CI covers the real + # 255-byte boundary verbatim. + long_name = "a" * 120 + ".md" + monkeypatch.setattr(file_memory, "_MAX_DESCRIPTION_FILE_NAME_BYTES", 130) + + store = FileSystemAgentFileStore(tmp_path) + provider = FileMemoryProvider(store=store, scope="user-1") + _, tools = await _prepare(provider) + save = tools["file_memory_write"] + list_files = tools["file_memory_ls"] + + short_name = "~memory-desc-" + hashlib.sha256(long_name.encode()).hexdigest() + for name, body, description in ( + (long_name, "long-body", "long-description"), + (short_name, "short-body", "short-description"), + ): + result = await save.invoke(arguments={"file_name": name, "content": body, "description": description}) + assert "with description" in _text(result) + assert (tmp_path / "user-1" / _description_file_name(name)).read_text(encoding="utf-8") == description + assert len(_description_file_name(name).encode("utf-8")) <= file_memory._MAX_DESCRIPTION_FILE_NAME_BYTES + + listed = {entry["name"]: entry["description"] for entry in json.loads(_text(await list_files.invoke()))} + assert listed == {long_name: "long-description", short_name: "short-description"} + await tools["file_memory_delete"].invoke(arguments={"file_name": short_name}) + listed_after = {entry["name"]: entry["description"] for entry in json.loads(_text(await list_files.invoke()))} + assert listed_after == {long_name: "long-description"} + + async def test_delete_removes_sidecar() -> None: """Deleting a file also removes its companion description sidecar.""" store = InMemoryAgentFileStore() From 1d770697d248ff3d68a632ac51262cfe2e276777 Mon Sep 17 00:00:00 2001 From: Lushiyao5 <321342421+Lushiyao5@users.noreply.github.com> Date: Tue, 3 Nov 2026 07:13:35 -0500 Subject: [PATCH 4/4] Python: match the reserved description-sidecar prefix case-insensitively On case-insensitive filesystems (default APFS and NTFS) an uppercase or mixed-case reserved prefix would bypass the startswith check and let a valid short memory alias a long memory's digest sidecar, so writing or deleting the short memory could clobber the long memory's description. Match the reserved prefix case-insensitively so case variants also take the digest branch, and add uppercase and mixed-case lifecycle regressions covering write, read, and delete. --- .../agent_framework/_harness/_file_memory.py | 5 +- .../tests/core/test_harness_file_memory.py | 47 +++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/python/packages/core/agent_framework/_harness/_file_memory.py b/python/packages/core/agent_framework/_harness/_file_memory.py index 83737d7fc58..28c83a35175 100644 --- a/python/packages/core/agent_framework/_harness/_file_memory.py +++ b/python/packages/core/agent_framework/_harness/_file_memory.py @@ -118,10 +118,13 @@ def _description_file_name(file_name: str) -> str: practice). Names that already start with the reserved digest prefix take the digest branch too: otherwise a short memory named after another name's digest stem would share that name's sidecar without any hash collision. + The prefix is matched case-insensitively because the default APFS and NTFS + filesystems are case-insensitive, so an uppercase or mixed-case reserved + prefix would otherwise alias a long name's digest sidecar on disk. """ sidecar = f"{file_name}{_DESCRIPTION_SUFFIX}" if ( - not file_name.startswith(_DIGEST_DESCRIPTION_PREFIX) + not file_name.lower().startswith(_DIGEST_DESCRIPTION_PREFIX) and len(sidecar.encode("utf-8")) <= _MAX_DESCRIPTION_FILE_NAME_BYTES ): return sidecar 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 9231e1ae8d9..5bb2d7bda52 100644 --- a/python/packages/core/tests/core/test_harness_file_memory.py +++ b/python/packages/core/tests/core/test_harness_file_memory.py @@ -98,6 +98,10 @@ def test_description_file_name_stays_within_filesystem_name_limits() -> None: assert _description_file_name(digest_stem) != digest_stem + "_description.md" assert _description_file_name(digest_stem) != _description_file_name(over_limit) assert _description_file_name("~memory-desc-x") != "~memory-desc-x_description.md" + # The reserved prefix is matched case-insensitively so case variants also + # take the digest branch and cannot alias a long name's digest sidecar. + for variant in ("~MEMORY-DESC-x", "~Memory-Desc-x"): + assert _description_file_name(variant) != variant + "_description.md" def test_is_internal_file_detects_sidecars_and_index() -> None: @@ -245,6 +249,49 @@ async def test_long_name_and_matching_digest_stem_short_name_have_independent_li assert listed_rewritten == {long_name: "long-description-2"} +@pytest.mark.parametrize("prefix_variant", ["~MEMORY-DESC-", "~Memory-Desc-"]) +async def test_case_variant_reserved_prefix_short_name_does_not_alias_long_name(prefix_variant: str) -> None: + """Upper/mixed-case reserved prefixes must not alias a long name's digest sidecar. + + On case-insensitive filesystems (default APFS/NTFS) a short memory whose + name uses an uppercase or mixed-case reserved prefix would otherwise + resolve to the same on-disk sidecar as a long name with the matching + digest, so writing or deleting it could clobber the long memory's + description without any SHA-256 collision. + """ + import hashlib + + store = InMemoryAgentFileStore() + provider = FileMemoryProvider(store=store, scope="user-1") + _, tools = await _prepare(provider) + save = tools["file_memory_write"] + read = tools["file_memory_read"] + delete = tools["file_memory_delete"] + list_files = tools["file_memory_ls"] + + long_name = "a" * 238 + ".md" + short_name = prefix_variant + hashlib.sha256(long_name.encode()).hexdigest() + assert short_name.startswith(prefix_variant) + # The case-variant reserved name must not share the long name's sidecar. + assert _description_file_name(short_name).lower() != _description_file_name(long_name).lower() + + await save.invoke(arguments={"file_name": long_name, "content": "long-body", "description": "long-description"}) + await save.invoke(arguments={"file_name": short_name, "content": "short-body", "description": "short-description"}) + + # Writing the case-variant memory must not overwrite the long memory's description. + listed = {entry["name"]: entry["description"] for entry in json.loads(_text(await list_files.invoke()))} + assert listed[long_name] == "long-description" + assert listed[short_name] == "short-description" + assert _text(await read.invoke(arguments={"file_name": long_name})) == "long-body" + assert _text(await read.invoke(arguments={"file_name": short_name})) == "short-body" + + # Deleting the case-variant memory must not remove the long memory's description. + await delete.invoke(arguments={"file_name": short_name}) + listed_after = {entry["name"]: entry["description"] for entry in json.loads(_text(await list_files.invoke()))} + assert listed_after == {long_name: "long-description"} + assert _text(await read.invoke(arguments={"file_name": long_name})) == "long-body" + + async def test_long_name_sidecar_is_writable_on_disk(tmp_path, monkeypatch) -> None: """A verbatim sidecar over the name limit must fall back to a writable digest on disk.