Repository navigation
Python: [Feature]: AgentFileStore.delete_directory for releasing empty explicit directories #9666
Description
Problem
The harness cleans up session scoped directories when a session ends. AgentFileStore.delete(path) is strictly file-only (a path naming a directory returns False, matching FileSystemAgentFileStore.delete on a real filesystem), so there is currently no public, backend independent way to release an explicitly created empty directory. InMemoryAgentFileStore tracks explicit directories separately from stored files, and without a release path those entries live for the lifetime of the store. This surfaced in review of #8687 and its resubmission #9608, where keeping delete() file-only required a companion operation to preserve the existing session cleanup behavior.
Use Case and Evidence
create_directory(path) is public and backend independent, but its counterpart is missing: after all files under a session directory are deleted, the empty explicit directory remains, and the in-memory store keeps its _explicit_dirs entry indefinitely. Real filesystems solve this with rmdir; the in-memory store needs an equivalent so backend substitution stays unobservable.
Desired Outcome
A public AgentFileStore.delete_directory(path) -> bool that releases an explicitly created, empty directory and returns True, and returns False when the path is missing, names a file, or still holds entries (files or nested explicit directories). The base class should ship a default implementation returning False so third-party stores keep working unchanged until they opt in.
Alternatives Considered
Overloading delete() to also remove empty directories: rejected in #8687 review because it makes backend substitution observable (os.remove raises IsADirectoryError while the in-memory store would have succeeded silently) and breaks the documented file-only contract of delete().
Leaking explicit directories until store teardown: the status quo; the memory growth is small but the asymmetry with create_directory invites bugs.
Proposed Direction
Python: delete_directory on the AgentFileStore base class (default False), InMemoryAgentFileStore removing the explicit-dir entry after checking it holds nothing, and FileSystemAgentFileStore calling rmdir and translating missing/file/nonempty into False. Implemented in #9608 with cross backend parity tests.
Cross language consistency needs a call from the .NET side: whether the .NET IAgentFileStore gains a matching method or whether directory cleanup stays Python-only for now. CC westey (@westey-m) for that decision.
Scope
Python: agent_framework._harness public store contract (and the .NET store contract if parity is wanted).
Metadata
Metadata
Labels
Type
Projects
- Status
Activity
[-][Feature]: AgentFileStore.delete_directory for releasing empty explicit directories[/-][+].NET: [Feature]: AgentFileStore.delete_directory for releasing empty explicit directories[/+][-].NET: [Feature]: AgentFileStore.delete_directory for releasing empty explicit directories[/-][+]Python: [Feature]: AgentFileStore.delete_directory for releasing empty explicit directories[/+]eavanvalkenburg commentedon Oct 24, 2026
/plan explore different approaches for the delete directory scenario
github-actions commentedon Oct 24, 2026
Summary
The issue asks for a public way to remove an empty directory that was made with
create_directory. It proposesAgentFileStore.delete_directory(path) -> bool. It would default toFalsein the base class, and the in-memory and filesystem stores would override it.What I found in the current code
InMemoryAgentFileStore.create_directoryis a no-op today. It has no explicit-directory tracking, so nothing leaks (_file_access.py). Directories are implied by file paths.main.delete()is file-only on both backends.create_directoryfor the file-memory working folder and for a session folder (_file_memory.py,_file_access.py~L2065). The .NET equivalent isFileMemoryProvider.cs.CreateDirectoryAsyncis also a no-op (source).Approaches for the delete-directory scenario
create_directorya no-op in memory, add no delete API. Directories stay implicit, as they are today. There is no leak and no new public surface. The cost is a difference between backends: the filesystem leaves empty directories on disk, and memory never has any. That difference is probably acceptable. Nothing in the repo needs "directory exists" to be observable.delete_directory(the issue's proposal). It is a symmetric API, and the defaultFalsekeeps third-party stores working. The cost is a new public contract that the in-memory store must track state for, with parity tests on both backends. It also forces a decision about the .NETAgentFileStore, which has abstract members, so adding one is source-compatible only if it isvirtual.delete()to remove empty directories. I would avoid this. The issue notes it breaks the file-only contract and makes backend swaps observable.delete_directory. This is more powerful, but riskier, and it is not what session cleanup needs. I would leave it out.Recommendation
Code changes are only warranted if empty-directory cleanup is a real requirement. The problem as written depends on explicit-directory tracking that
maindoes not have. First decide whether that tracking is needed at all.create_directoryis a no-op for in-memory stores.Proposed plan (only if approach 2 is chosen)
No changes have been made. This is a proposal.
async def delete_directory(self, path) -> booltoAgentFileStore(non-abstract, defaultFalse), with a docstring that states the contract: an empty explicit directory returnsTrue; a missing path, a file, or a non-empty directory returnsFalse.delete_directory. A directory counts as non-empty if it holds files or nested explicit directories. Also makelist_childrenand other readers consistent with this tracking. Mind the case-insensitive keys.rmdirthrough the existing safe-path resolver, and map missing, file, and non-empty errors toFalse.@westey-m. Either add avirtual Task<bool> DeleteDirectoryAsyncwith a default offalse, or document it as Python-only for now. Cross-language drift in the file store contract should be a conscious choice.delete()on a directory still returnsFalse, and that a custom store that does not override the method returnsFalse.The issue is resolved when both built-in stores give identical results for those cases and no explicit-directory state remains after session cleanup.
Question for the maintainers
Is explicit-directory tracking in the in-memory store a goal in its own right? If not, approach 1 makes this feature unnecessary.