fix(io): make InMemoryFileStore.list respect directory boundaries#3977
Draft
tomsen-ai wants to merge 1 commit into
Draft
fix(io): make InMemoryFileStore.list respect directory boundaries#3977tomsen-ai wants to merge 1 commit into
tomsen-ai wants to merge 1 commit into
Conversation
InMemoryFileStore.list used bare string prefix matching, which diverged from LocalFileStore (documented as S3-consistent) in two ways: - listing 'a/b' also matched sibling keys like 'a/bc.txt' and invented phantom subdirectories from their suffixes - listing an exact file path raised IndexError instead of returning [path] like LocalFileStore does Normalize the prefix to end with '/', short-circuit exact file hits, and skip empty suffixes. Both stores implement the same FileStore interface and now agree on these inputs. Signed-off-by: tomsen-ai <230283659+tomsen-ai@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
HUMAN:
The in-memory file store's list() disagreed with the local (disk) one:it invented phantom directories from sibling names, and crashed with an
IndexError when given an exact file path. I went through the
side-by-side repro before and after the fix myself.
AGENT:
Why
InMemoryFileStore.listuses bare string prefix matching (file.startswith(path)+removeprefix), which is not aware of the/directory boundary. It diverges fromLocalFileStore.list(whose implementation is explicitly commented as S3-consistent behavior) in two observable ways:sessions/abc/event.jsonandsessions/abcdef/other.json,list("sessions/abc")returns a phantomsessions/abc/def/entry derived from the sibling's suffix.LocalFileStorereturns only the real children.list("sessions/abc/event.json")raisesIndexError: list index out of range(suffix=""→parts=[]→parts[0]).LocalFileStorereturns[path].Both classes implement the same
FileStoreinterface, so consumers switching between the in-memory and local backends currently see different results for identical inputs.The in-repo consumer (
conversation/event_store.py) happens to be shielded by itsEVENT_NAME_REfilename filter and fixed-width event IDs, so this is not currently user-visible there — but the divergence is a trap for any otherFileStoreconsumer.Summary
[path], matchingLocalFileStore./so listinga/bcannot match sibling keys likea/bc.txt."") behavior unchanged.Issue Number
N/A (searched existing issues/PRs: #3163/#3146 are about LRU eviction in this class, not
listsemantics).How to Test
New tests fail on main:
With this change:
tests/sdk/io/26 passed. Downstream consumer regression:tests/sdk/conversation708 passed.Video/Screenshots
Side-by-side on main (same fixture, same call):
After this change the two backends return identical results for all cases in the new test suite.