Skip to content

Python: [Bug]: InMemoryAgentFileStore permits a path to be both a file and a directory #8567

Description

Description

InMemoryAgentFileStore permits both Reports and Reports/q1.txt to exist. Consequently, list_children("") returns two entries named Reports, one reported as a file and one as a directory. A caller cannot tell which path to use, and this state cannot occur in FileSystemAgentFileStore.

This happens in either write order. write() checks only whether the exact file key exists; it does not reject a file ancestor or an existing descendant. Its case-insensitive key normalization means Reports and reports/q1.txt conflict as well.

Expected: Reject the second write without changing the existing content. A file path cannot also serve as a directory, regardless of casing.

Actual: Both writes succeed and the root listing contains a same-named file and directory.

Code Sample

import asyncio
from agent_framework import InMemoryAgentFileStore


async def main() -> None:
    store = InMemoryAgentFileStore()
    await store.write("Reports", "file")
    await store.write("Reports/q1.txt", "nested")
    print([(entry.name, entry.type) for entry in await store.list_children()])
    # [('Reports', 'directory'), ('Reports', 'file')]


asyncio.run(main())

The new regression test fails against the unmodified package (Failed: DID NOT RAISE NotADirectoryError) and passes against the local fix.

Error Messages / Stack Traces

None. The inconsistent state is accepted silently.

Package Versions

Source checkout at 999dda7970fe969c0901d524365ce34ecbda6227 (agent-framework-core version 1.18.0); the relevant write() implementation was also checked against the repository's current default branch on 2026-09-21. The public API reproduction and regression tests used this source checkout through an existing virtual environment, not a separately installed release package.

Python Version

Python 3.12.0 for the isolated source execution.

Additional Context

FileSystemAgentFileStore uses real filesystem paths, where a file cannot be the parent of another file and a nonempty directory cannot be replaced by a file. The in-memory store is used for tests and lightweight use, so matching this basic path invariant avoids testing a state the disk store cannot represent.

Local draft fix: reject a write if any ancestor is already a file or if the target path already has descendants. A focused regression test covers both write orders and verifies the existing content remains intact. The new test fails on the baseline and passes with the fix. The full test_harness_file_access.py file passes with the fix: 73 passed, 6 skipped. Ruff check, Ruff format check, py_compile, and git diff --check also pass. The reused virtual environment lacks pytest-timeout, causing an unrelated pytest configuration warning.

Searches of repository issues and PRs for InMemoryAgentFileStore conflict, file and directory same name, and file store path collision found no matching report or implementation on 2026-09-21.

Activity

  1. added
    pythonUsage: [Issues, PRs], Target: Python
    triageUsage: [Issues], Target: All issues that still need to be triaged
    on Sep 21, 2026
  2. added
    reproducedUsage: [Issues], Target: all issues that can be reproduced by the triage workflow
    on Sep 21, 2026
  3. github-actions commented on Sep 21, 2026

    @github-actions
    Contributor

    🤖 Automated triage reproduction notes (agent-authored — trust but verify)

    Agent analysis

    Repro: python/packages/core/agent_framework/_harness/_file_access.py::InMemoryAgentFileStore.write at lines 1046-1058 accepts a target whose normalized case-insensitive path has either an existing file ancestor or existing descendants. Writing Reports and Reports/q1.txt in either order triggers the bug, after which list_children() can return Reports as both a directory and file. Minimal repro: instantiate the store, write either path, then assert the second write raises NotADirectoryError; both parameterized orders currently fail with DID NOT RAISE.

    • Failing test: python/packages/core/tests/core/test_harness_file_access.py::test_in_memory_store_rejects_file_directory_path_collisions
    • Files examined: python/AGENTS.md, python/packages/core/AGENTS.md, python/packages/core/agent_framework/_harness/_file_access.py, python/packages/core/tests/core/test_harness_file_access.py, python/packages/core/pyproject.toml, REPRODUCTION_REPORT.md
    • Tests run: test_in_memory_store_list_directories, test_in_memory_store_write_file_raises_when_exists_and_no_overwrite, test_in_memory_store_rejects_file_directory_path_collisions, public API listing reproduction, FileSystemAgentFileStore collision comparison
    • Reported version: 1.18.0
    • Current version: 1.19.0
  4. added
    harness[Issues, PRs], Target: harness-level items
    and removed
    triageUsage: [Issues], Target: All issues that still need to be triaged
    on Sep 21, 2026
  5. manjunathshiva commented on Sep 21, 2026

    @manjunathshiva
    Contributor

    Reproduced on main (53e98134c) — writing Reports and then Reports/q1.txt both succeed, and list_children("") comes back with two entries named Reports, one directory and one file:

    MEM  Reports              -> OK
    MEM  Reports/q1.txt       -> OK
    MEM  children             -> [('Reports', 'directory'), ('Reports', 'file')]
    

    Two things worth settling before anyone writes the fix, though.

    Is this intended? The class docstring says the quiet part out loud — "Directory concepts are simulated using path prefixes — no explicit directory structure is maintained" (_file_access.py:1027). The .NET InMemoryAgentFileStore carries that same sentence, and its WriteAsync is a bare this._files[path] = content; over an OrdinalIgnoreCase dictionary, so it behaves identically. create_directory is also a no-op here, so the store already cannot represent an empty directory. That reads like a deliberate simplification rather than an oversight, and fixing only Python would split the two implementations apart.

    If it should be enforced, the exception needs deciding, and NotADirectoryError would not match FileSystemAgentFileStore. I measured the disk store rather than assuming, on Windows:

    order disk store raises
    Reports, then Reports/q1.txt FileExistsError (WinError 183), from parent.mkdir(exist_ok=True)
    Reports/q1.txt, then Reports PermissionError (Errno 13)

    So the disk store raises neither NotADirectoryError nor a consistent type — it differs by write order, and the second case is platform-dependent, so "match the disk store" is not actually a well-defined target.

    FileExistsError would also be an actively bad choice: file_access_write already catches it (_file_access.py:1919) and tells the model "File 'X' already exists. To replace it, write again with overwrite set to true." — advice that would not work, since overwrite=True does not resolve a file/directory collision.

    So: westey (@westey-m), is the collision an accepted consequence of the documented prefix simulation, or should the in-memory store enforce the invariant? If the latter, which exception, and should the .NET store change in the same PR to keep the two aligned?

    Happy to implement whichever way you decide — an ancestor-is-a-file check plus a descendant-prefix scan in write(), with tests covering both write orders and the case-insensitive variant. 林黛玉倒拔垂杨柳 (@boshilin123) nice find; the duplicate-name output from list_children is the part that makes it more than cosmetic.

  6. westey-m commented on Sep 21, 2026

    @westey-m
    Collaborator

    Manjunath Janardhan (@manjunathshiva), thanks for the offer to implement. I think we should just stick to matching what a regular file system does, which is not allowing both a file and a directory with the same name to exist. The abstraction doesn't currently define a clear protocol around what exceptions should be raised for these cases, but we really should add that, and ensure that each implementation matches the common exception types.

  7. manjunathshiva commented on Sep 22, 2026

    @manjunathshiva
    Contributor

    Thanks — matching a regular file system is a clear answer, and it settles the part I could not decide. Four things I ran into while scoping it, because the exception protocol turns out to reach further than InMemoryAgentFileStore.

    1. Proposed mapping. FileExistsError is already taken by a different contract — AgentFileStore.write documents that an implementation "must perform an atomic exclusive create and raise FileExistsError" when overwrite=False. Reusing it for a collision would put two unrelated meanings on one type, so:

    case Python .NET
    an ancestor of the target is a file (Reports is a file, write Reports/q1.txt) NotADirectoryError (ENOTDIR) IOException
    the target already has descendants (Reports/q1.txt exists, write Reports) IsADirectoryError (EISDIR) IOException

    Python has distinct classes for both errnos; .NET has no equivalents, so "common exception types" can only mean equivalent semantics with language-idiomatic types, not literally the same type. If you would rather .NET carried distinct types too, that is a new-public-type decision and I would want your call before adding any.

    2. Case sensitivity is not determined by "match a regular file system", and it changes the behaviour. Both in-memory stores are case-insensitive — Python lowercases keys, .NET uses StringComparer.OrdinalIgnoreCase. A POSIX file system is case-sensitive. So Reports plus reports/q1.txt collides in the in-memory stores and on Windows, but is legal on Linux. Should collision detection follow the store's existing case-insensitive keys, or the case sensitivity of the underlying platform for FileSystemAgentFileStore? The second makes the two implementations disagree by platform, which I assume is not what you want, but it is the more literal reading of "what a regular file system does".

    3. The filesystem stores need work too, which I do not think is obvious from the issue. FileSystemAgentFileStore only translates OSError for path validation (symlink and escape checks, re-raised as ValueError); write-time OS errors pass straight through. Measured on Windows:

    • Reports then Reports/q1.txt → FileExistsError (WinError 183), out of the full_path.parent.mkdir(parents=True, exist_ok=True) at _file_access.py:1319
    • Reports/q1.txt then Reports → PermissionError (Errno 13); POSIX would differ

    So "ensure each implementation matches the common exception types" means normalising OS errors in the filesystem stores as well — four implementations across the two languages, plus conformance coverage. Worth noting the .NET abstraction currently documents no exceptions at all (AgentFileStore has zero <exception> tags) and AgentFileStoreContractTests asserts only ArgumentException, ArgumentNullException and OperationCanceledException, so .NET would be gaining its first documented exception contract here.

    4. The tool-level handler has to change in the same PR, or this becomes a worse bug than it fixes. FileAccessProvider.file_access_write catches specific types and returns a string to the model. A new NotADirectoryError would not be caught, so it would escape and fail the whole agent run — where today the model just sees a confusing listing. It also currently catches FileExistsError and answers "write again with overwrite set to true", which cannot resolve a file/directory collision. So the handler needs to catch the new types and return something actionable, e.g. that the path is a directory and a different name is needed.

    Shape. Two PRs rather than one, matching how the harness changes have gone before (#6474/#6476, #6801/#6807, #7669/#7671) and because commit titles here carry a language prefix: Python first, defining the protocol on the abstraction with the Raises:/"must" wording already used for the overwrite=False case, then .NET mirroring it with contract tests. Happy to do both.

    Two smaller scoping questions: create_directory is a no-op in the in-memory store ("directories are implicit from file paths"), so it cannot create a collision but also cannot reserve a name — in scope or not? And the descendant check is an O(n) key scan per write, which seems fine for a test-oriented store but I would rather say so than have it found in review.

    Also flagging that this is an intentional break of documented behaviour: the permissive collision is currently described as deliberate in both languages, in the same sentence — "Directory concepts are simulated using path prefixes — no explicit directory structure is maintained" — so those docstrings change too. Every previous paired FileAccess change in this area shipped as [BREAKING] (#6474/#6476, #6801/#6807, #7669/#7671), so I would expect these to carry the label as well; tell me if you would rather they did not.

  8. lakers-abing commented on Sep 23, 2026

    @lakers-abing

    Hi 林黛玉倒拔垂杨柳 (@boshilin123), I saw the maintainer discussion and your note that you're happy to handle the Python and .NET work. I have a separate Python implementation with regression tests for both write orders and case-insensitive collisions. Are you planning to submit the Python-side PR yourself, or would you be comfortable with me opening a separate PR for that part? I want to avoid duplicating your work.

  9. manjunathshiva commented on Sep 23, 2026

    @manjunathshiva
    Contributor

    lakers-abing that offer to handle the Python and .NET sides was mine rather than 林黛玉倒拔垂杨柳 (@boshilin123)'s — easy to miss in a long thread. Either way, please go ahead with the Python PR; you have code and tests already and I have not written a line, so there is nothing to duplicate.

    Three things from scoping it that may save you a review cycle, since they are the parts that bite outside the in-memory store.

    The tool-level handler needs changing in the same PR, or the fix is a regression. FileAccessProvider.file_access_write catches specific exception types and returns a string to the model (_file_access.py:1919). A new NotADirectoryError or IsADirectoryError would not be caught there, so it escapes and fails the whole agent run — where today the model just gets a confusing listing. That handler also currently answers FileExistsError with "write again with overwrite set to true", which cannot resolve a file/directory collision, so it needs a branch that tells the model the path is a directory and to choose another name.

    FileExistsError is already taken. AgentFileStore.write documents that an implementation "must perform an atomic exclusive create and raise FileExistsError" when overwrite=False, so reusing it for a collision would put two unrelated meanings on one type.

    Case sensitivity is not settled, and your tests encode an answer to it. Both in-memory stores are case-insensitive — Python lowercases the key, .NET uses StringComparer.OrdinalIgnoreCase — while a POSIX file system is case-sensitive. So Reports plus reports/q1.txt collides in the in-memory store and on Windows but is legal on Linux. westey (@westey-m) said "match a regular file system", which does not by itself decide this, and I asked him about it but have not had an answer yet. Worth pinning before your tests bake it in.

    One scope note in case it affects how you frame the PR: FileSystemAgentFileStore only translates OSError for path validation, so write-time OS errors pass straight through. I measured it on Windows — Reports then Reports/q1.txt raises FileExistsError (WinError 183) out of the parent.mkdir at _file_access.py:1319, and the reverse order raises PermissionError (Errno 13). So "each implementation matches the common exception types" reaches the filesystem store too, not just the in-memory one. Whether that belongs in your PR or a follow-up is worth asking westey (@westey-m).

    Happy to review when you open it.

  10. ManoharPaturi commented on Oct 22, 2026

    @ManoharPaturi
    Contributor

    The fix for this issue is resubmitted as #9608 (replacing the closed #8687, which was closed with a request to address the outstanding review feedback about preserving the file-only delete contract). The new PR contains the original path-collision fix plus that feedback: delete() is now strictly file-only, and a new delete_directory() method releases empty explicitly created directories so session-scoped cleanup still works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

harness[Issues, PRs], Target: harness-level itemspythonUsage: [Issues, PRs], Target: PythonreproducedUsage: [Issues], Target: all issues that can be reproduced by the triage workflow

Type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions