From 05f1554d6486fb9c4c6825c9ed0c1bb36e62c729 Mon Sep 17 00:00:00 2001 From: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com> Date: Wed, 23 Sep 2026 18:49:36 +0530 Subject: [PATCH 1/4] Python: fix(core): prevent path collisions between files and directories in InMemoryAgentFileStore Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com> --- .../agent_framework/_harness/_file_access.py | 22 ++++++++- .../tests/core/test_harness_file_access.py | 49 +++++++++++++++++++ 2 files changed, 69 insertions(+), 2 deletions(-) diff --git a/python/packages/core/agent_framework/_harness/_file_access.py b/python/packages/core/agent_framework/_harness/_file_access.py index 6191bb98f7d..2b0e885d143 100644 --- a/python/packages/core/agent_framework/_harness/_file_access.py +++ b/python/packages/core/agent_framework/_harness/_file_access.py @@ -1079,7 +1079,16 @@ async def write(self, path: str, content: str, *, overwrite: bool = True) -> Non """ display = _normalize_relative_path(path) key = display.lower() + dir_prefix = f"{key}/" async with self._lock: + parts = key.split("/") + for i in range(1, len(parts)): + ancestor = "/".join(parts[:i]) + if ancestor in self._files: + raise NotADirectoryError(f"Not a directory: {path!r}") + for existing_key in self._files: + if existing_key.startswith(dir_prefix): + raise IsADirectoryError(f"Is a directory: {path!r}") if not overwrite and key in self._files: raise FileExistsError(f"File already exists: {path!r}") self._files[key] = (display, content) @@ -1191,8 +1200,17 @@ def scan() -> list[FileSearchResult]: return await _run_search_with_timeout(asyncio.to_thread(scan)) async def create_directory(self, path: str) -> None: - """No-op: directories are implicit from file paths in the in-memory store.""" - del path + """Ensure the directory at ``path`` exists.""" + display = _normalize_relative_path(path, is_directory=True) + key = display.lower() + if not key: + return + async with self._lock: + parts = key.split("/") + for i in range(1, len(parts) + 1): + ancestor = "/".join(parts[:i]) + if ancestor in self._files: + raise FileExistsError(f"File already exists: {ancestor!r}") @experimental(feature_id=ExperimentalFeature.HARNESS) diff --git a/python/packages/core/tests/core/test_harness_file_access.py b/python/packages/core/tests/core/test_harness_file_access.py index 8c01b1c6146..f0f97a64a55 100644 --- a/python/packages/core/tests/core/test_harness_file_access.py +++ b/python/packages/core/tests/core/test_harness_file_access.py @@ -308,6 +308,55 @@ async def test_in_memory_store_normalizes_paths() -> None: await store.write(bad, "boom") +async def test_in_memory_store_rejects_file_ancestor_collision() -> None: + """Writing a file whose ancestor is already a file must raise NotADirectoryError.""" + store = InMemoryAgentFileStore() + await store.write("Reports", "file content") + + with pytest.raises(NotADirectoryError): + await store.write("Reports/q1.txt", "nested content") + + with pytest.raises(NotADirectoryError): + await store.write("reports/deep/q2.txt", "deep nested content") + + assert await store.read("Reports") == "file content" + children = await store.list_children("") + assert len(children) == 1 + assert children[0].name == "Reports" + assert children[0].type == FileStoreEntry.FILE + + +async def test_in_memory_store_rejects_descendant_directory_collision() -> None: + """Writing a file whose path is already a directory must raise IsADirectoryError.""" + store = InMemoryAgentFileStore() + await store.write("Reports/q1.txt", "nested content") + + with pytest.raises(IsADirectoryError): + await store.write("Reports", "file content") + + with pytest.raises(IsADirectoryError): + await store.write("reports", "case insensitive collision") + + assert await store.read("Reports/q1.txt") == "nested content" + assert await store.read("Reports") is None + children = await store.list_children("") + assert len(children) == 1 + assert children[0].name == "Reports" + assert children[0].type == FileStoreEntry.DIRECTORY + + +async def test_in_memory_store_create_directory_rejects_existing_file() -> None: + """Creating a directory that collides with an existing file must raise FileExistsError.""" + store = InMemoryAgentFileStore() + await store.write("Reports", "file content") + + with pytest.raises(FileExistsError): + await store.create_directory("Reports") + + with pytest.raises(FileExistsError): + await store.create_directory("reports/nested") + + async def test_filesystem_store_round_trips_files(tmp_path: Path) -> None: """The filesystem store should round-trip files on disk and create parents on write.""" store = FileSystemAgentFileStore(tmp_path) From 9b05a0f91616add523e40fc92b920ea8a31617bf Mon Sep 17 00:00:00 2001 From: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com> Date: Fri, 25 Sep 2026 18:43:36 +0530 Subject: [PATCH 2/4] Index directory prefixes and persist explicit directories in InMemoryAgentFileStore Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com> --- .../agent_framework/_harness/_file_access.py | 73 +++++++++++++++---- .../tests/core/test_harness_file_access.py | 60 +++++++++++++++ 2 files changed, 120 insertions(+), 13 deletions(-) diff --git a/python/packages/core/agent_framework/_harness/_file_access.py b/python/packages/core/agent_framework/_harness/_file_access.py index 2b0e885d143..5c256365a45 100644 --- a/python/packages/core/agent_framework/_harness/_file_access.py +++ b/python/packages/core/agent_framework/_harness/_file_access.py @@ -1064,12 +1064,24 @@ def __init__(self) -> None: # wrote, mirroring how :class:`FileSystemAgentFileStore` preserves the # on-disk casing. self._files: dict[str, tuple[str, str]] = {} + self._dir_counts: dict[str, int] = {} + self._explicit_dirs: dict[str, str] = {} self._lock = asyncio.Lock() @staticmethod def _key(path: str) -> str: return _normalize_relative_path(path).lower() + @staticmethod + def _ancestor_keys(key: str) -> list[str]: + """Return all strict ancestor directory keys in top-down order in linear time.""" + ancestors: list[str] = [] + idx = key.find("/") + while idx != -1: + ancestors.append(key[:idx]) + idx = key.find("/", idx + 1) + return ancestors + async def write(self, path: str, content: str, *, overwrite: bool = True) -> None: """Write ``content`` to the file at ``path``. @@ -1079,19 +1091,20 @@ async def write(self, path: str, content: str, *, overwrite: bool = True) -> Non """ display = _normalize_relative_path(path) key = display.lower() - dir_prefix = f"{key}/" async with self._lock: - parts = key.split("/") - for i in range(1, len(parts)): - ancestor = "/".join(parts[:i]) + ancestors = self._ancestor_keys(key) + for ancestor in ancestors: if ancestor in self._files: raise NotADirectoryError(f"Not a directory: {path!r}") - for existing_key in self._files: - if existing_key.startswith(dir_prefix): - raise IsADirectoryError(f"Is a directory: {path!r}") + if self._dir_counts.get(key, 0) > 0 or key in self._explicit_dirs: + raise IsADirectoryError(f"Is a directory: {path!r}") if not overwrite and key in self._files: raise FileExistsError(f"File already exists: {path!r}") + is_new_file = key not in self._files self._files[key] = (display, content) + if is_new_file: + for ancestor in ancestors: + self._dir_counts[ancestor] = self._dir_counts.get(ancestor, 0) + 1 async def read(self, path: str) -> str | None: """Return the file content, or ``None`` if the file does not exist.""" @@ -1104,7 +1117,16 @@ async def delete(self, path: str) -> bool: """Delete the file and return whether anything was removed.""" key = self._key(path) async with self._lock: - return self._files.pop(key, None) is not None + removed = self._files.pop(key, None) + if removed is not None: + for ancestor in self._ancestor_keys(key): + new_count = self._dir_counts[ancestor] - 1 + if new_count <= 0: + del self._dir_counts[ancestor] + else: + self._dir_counts[ancestor] = new_count + return True + return False async def list_children(self, directory: str = "") -> list[FileStoreEntry]: """Return the direct child files and subdirectories of ``directory``. @@ -1124,11 +1146,12 @@ async def list_children(self, directory: str = "") -> list[FileStoreEntry]: prefix += "/" prefix_depth = prefix.count("/") async with self._lock: - entries = [(key, display) for key, (display, _) in self._files.items()] + file_entries = [(key, display) for key, (display, _) in self._files.items()] + dir_entries = list(self._explicit_dirs.items()) files: list[str] = [] directories: list[str] = [] seen_dirs: set[str] = set() - for key, display in entries: + for key, display in file_entries: if not key.startswith(prefix): continue # Unicode lowercasing can change character counts, so key offsets @@ -1143,6 +1166,17 @@ async def list_children(self, directory: str = "") -> list[FileStoreEntry]: continue seen_dirs.add(segment_key) directories.append(segment) + for key, display in dir_entries: + if not key.startswith(prefix) or key == prefix.rstrip("/"): + continue + remainder = display.split("/", prefix_depth)[-1] + segment, _, _ = remainder.partition("/") + if segment: + segment_key = segment.lower() + if segment_key in seen_dirs: + continue + seen_dirs.add(segment_key) + directories.append(segment) results: list[FileStoreEntry] = [FileStoreEntry(name, FileStoreEntry.DIRECTORY) for name in directories] results.extend(FileStoreEntry(name, FileStoreEntry.FILE) for name in files) return results @@ -1206,11 +1240,24 @@ async def create_directory(self, path: str) -> None: if not key: return async with self._lock: - parts = key.split("/") - for i in range(1, len(parts) + 1): - ancestor = "/".join(parts[:i]) + ancestors = self._ancestor_keys(key) + for ancestor in ancestors: if ancestor in self._files: raise FileExistsError(f"File already exists: {ancestor!r}") + if key in self._files: + raise FileExistsError(f"File already exists: {key!r}") + if key not in self._explicit_dirs: + idx_display = 0 + for ancestor in ancestors: + next_slash = display.find("/", idx_display) + if next_slash != -1: + ancestor_display = display[:next_slash] + idx_display = next_slash + 1 + else: + ancestor_display = display + if ancestor not in self._explicit_dirs: + self._explicit_dirs[ancestor] = ancestor_display + self._explicit_dirs[key] = display @experimental(feature_id=ExperimentalFeature.HARNESS) diff --git a/python/packages/core/tests/core/test_harness_file_access.py b/python/packages/core/tests/core/test_harness_file_access.py index f0f97a64a55..dd30a181168 100644 --- a/python/packages/core/tests/core/test_harness_file_access.py +++ b/python/packages/core/tests/core/test_harness_file_access.py @@ -357,6 +357,66 @@ async def test_in_memory_store_create_directory_rejects_existing_file() -> None: await store.create_directory("reports/nested") +async def test_in_memory_store_explicit_directory_reserves_path_against_write() -> None: + """Explicitly created directory must reserve the path and reject file writes.""" + store = InMemoryAgentFileStore() + await store.create_directory("Reports") + + with pytest.raises(IsADirectoryError): + await store.write("Reports", "file content") + + with pytest.raises(IsADirectoryError): + await store.write("reports", "case insensitive collision") + + children = await store.list_children("") + assert len(children) == 1 + assert children[0].name == "Reports" + assert children[0].type == FileStoreEntry.DIRECTORY + + nested_children = await store.list_children("Reports") + assert len(nested_children) == 0 + + +async def test_in_memory_store_explicit_nested_directory_creation() -> None: + """Explicit nested directory creation registers intermediate directories and preserves casing.""" + store = InMemoryAgentFileStore() + await store.create_directory("Reports/2026/Q1") + + root_children = await store.list_children("") + assert len(root_children) == 1 + assert root_children[0].name == "Reports" + assert root_children[0].type == FileStoreEntry.DIRECTORY + + reports_children = await store.list_children("Reports") + assert len(reports_children) == 1 + assert reports_children[0].name == "2026" + assert reports_children[0].type == FileStoreEntry.DIRECTORY + + q1_children = await store.list_children("Reports/2026") + assert len(q1_children) == 1 + assert q1_children[0].name == "Q1" + assert q1_children[0].type == FileStoreEntry.DIRECTORY + + with pytest.raises(IsADirectoryError): + await store.write("reports/2026", "collision") + + +async def test_in_memory_store_delete_file_cleans_up_implicit_directory_counts() -> None: + """Deleting a file unreserves implicit parent directories when no other files remain.""" + store = InMemoryAgentFileStore() + await store.write("data/temp/file.txt", "file content") + + with pytest.raises(IsADirectoryError): + await store.write("data/temp", "should fail while file exists") + + deleted = await store.delete("data/temp/file.txt") + assert deleted is True + + # Now that the implicit descendant is gone and temp was never explicitly created, writing succeeds + await store.write("data/temp", "now a file") + assert await store.read("data/temp") == "now a file" + + async def test_filesystem_store_round_trips_files(tmp_path: Path) -> None: """The filesystem store should round-trip files on disk and create parents on write.""" store = FileSystemAgentFileStore(tmp_path) From 92785b5d75728d4eaf8b3f8a568aa3cf211d0317 Mon Sep 17 00:00:00 2001 From: Manohar Paturi Date: Fri, 2 Oct 2026 15:44:19 +0530 Subject: [PATCH 3/4] test(harness): account for session dir in nested-path rejection test The upstream #8507 assertion expected list_children("") to stay empty, which held while create_directory was a no-op. This PR makes explicit directories listable, so the session-1 directory created during before_run now appears at the root. The invariant the test protects, that a rejected nested write leaves nothing behind, is preserved: the session directory exists and stays empty, with no notes child. --- .../packages/core/tests/core/test_harness_file_memory.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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 4f2e810c858..c5985df4032 100644 --- a/python/packages/core/tests/core/test_harness_file_memory.py +++ b/python/packages/core/tests/core/test_harness_file_memory.py @@ -305,8 +305,11 @@ async def test_tools_reject_nested_paths() -> None: saved = await tools["file_memory_write"].invoke(arguments={"file_name": "notes/plan.md", "content": "x"}) assert "subdirectory" in _text(saved) - # Nothing should have been written for the nested name. - assert await store.list_children("") == [] + # Nothing should have been written for the nested name. The session + # directory created during before_run is the only root entry, and the + # rejected write leaves it empty (no "notes" child). + assert [entry.name for entry in await store.list_children("")] == ["session-1"] + assert await store.list_children("session-1") == [] # Backslash separators are normalized to "/" and rejected the same way. saved_backslash = await tools["file_memory_write"].invoke(arguments={"file_name": "notes\\plan.md", "content": "x"}) From b4bf58ee38a52ebe2a147021b78c7bdc7cdcbf62 Mon Sep 17 00:00:00 2001 From: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com> Date: Wed, 14 Oct 2026 14:03:01 +0530 Subject: [PATCH 4/4] Python: let the in-memory store delete empty explicit directories Explicitly created directories had no deletion path, so session-scoped runs kept accumulating entries in the shared map. delete() now removes an explicit directory once nothing is stored under it and no other explicit directory is nested below, giving session directories a lifecycle. Non-empty directories stay put, matching the filesystem store's file-only deletes. Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com> --- .../agent_framework/_harness/_file_access.py | 16 +++++++++++++++- .../core/tests/core/test_harness_file_access.py | 17 +++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/python/packages/core/agent_framework/_harness/_file_access.py b/python/packages/core/agent_framework/_harness/_file_access.py index 57633afeb15..b98e029c267 100644 --- a/python/packages/core/agent_framework/_harness/_file_access.py +++ b/python/packages/core/agent_framework/_harness/_file_access.py @@ -1117,7 +1117,13 @@ async def read(self, path: str) -> str | None: return entry[1] if entry is not None else None async def delete(self, path: str) -> bool: - """Delete the file and return whether anything was removed.""" + """Delete the file and return whether anything was removed. + + An explicitly created directory can also be deleted, but only when it is + empty: nothing is stored under it and no other explicit directory is + nested below it. This gives session-scoped directories a lifecycle instead + of staying in the shared map forever. + """ key = self._key(path) async with self._lock: removed = self._files.pop(key, None) @@ -1129,6 +1135,14 @@ async def delete(self, path: str) -> bool: else: self._dir_counts[ancestor] = new_count return True + if key in self._explicit_dirs: + if self._dir_counts.get(key, 0) > 0: + return False + prefix = key + "/" + if any(other != key and other.startswith(prefix) for other in self._explicit_dirs): + return False + del self._explicit_dirs[key] + return True return False async def list_children(self, directory: str = "") -> list[FileStoreEntry]: diff --git a/python/packages/core/tests/core/test_harness_file_access.py b/python/packages/core/tests/core/test_harness_file_access.py index 14b26ba50cd..6f007812f7d 100644 --- a/python/packages/core/tests/core/test_harness_file_access.py +++ b/python/packages/core/tests/core/test_harness_file_access.py @@ -2719,3 +2719,20 @@ async def test_scope_alone_enables_scoped_mode(chat_client_base: SupportsChatGet provider = FileAccessProvider(store=store, scope="tenant-1") context = SessionContext(session_id=None, input_messages=[]) await provider.before_run(agent=None, session=AgentSession(), context=context, state={}) + + +async def test_in_memory_store_delete_removes_empty_explicit_directory() -> None: + """An empty explicitly created directory can be deleted; a used one cannot.""" + store = InMemoryAgentFileStore() + await store.create_directory("sessions/s1") + + assert await store.delete("sessions/s1") is True + children = await store.list_children("sessions") + assert all(entry.name != "s1" for entry in children) + + await store.create_directory("sessions/s2") + await store.write("sessions/s2/state.txt", "data") + assert await store.delete("sessions/s2") is False + assert await store.delete("sessions/s2/state.txt") is True + assert await store.delete("sessions/s2") is True + assert await store.delete("sessions") is True