Fix stale S3FileSystem listing and object caches

[laughingman7743]

WHAT

• S3FileSystem.invalidate_cache(path) now also drops the (path, "/") and (path, "") listing entries that _ls_dirs() stores, for the path and each parent path.
  rm(), touch(), pipe_file(), S3File.commit(), mv()/cp() and the other writes call it, so after a write through the filesystem, ls() and find() on the directory or an ancestor list from S3 again.
• _ls_dirs() no longer reads or writes the cache when it gets a prefix or next_token.
  Before this change, fs.find(d) followed by fs.find(d, prefix="x") returned the cached, unfiltered listing of d.
• _ls_dirs() now evicts the cached listing when a complete listing comes back empty.
  Before, ls(d, refresh=True) returned [] after the last object was deleted elsewhere, but the next ls(d) returned the deleted objects from the cache again.
• find(withdirs=True) without maxdepth no longer extends the cached listing in place.
  Before, each repeated call appended the derived directories to the cache again, so later results contained duplicates.
• find(path, refresh=True) now lists from S3. Before, _find() did not forward refresh to _ls_dirs(), its info() fallback, or the recursive maxdepth calls, so it returned the cached listings.
• A refreshed HeadObject or HeadBucket that finds nothing now evicts the cached entry.
  Before, ls(file, refresh=True) returned [] after the file was deleted elsewhere, but the next ls(file), info(file) or exists(file) returned it from the cache. The same happened for a deleted bucket after info(bucket, refresh=True), including when the bucket was still in the cached bucket listing (that listing is now evicted only if it lists the missing bucket).
• exists(path, refresh=True) now queries S3. Before, exists() ignored its keyword arguments and answered from the cache. fsspec's exists() passes them to info().
• Adds Google-style docstrings to _ls_dirs() and invalidate_cache(), and documents prefix/refresh of find() and refresh of exists(). The invalidate_cache docstring matches the one in Document every public API in pyathena/ and check docstrings with ruff #919.

S3FileSystemAsync shares the sync dircache and delegates invalidate_cache, _ls and _find to the sync filesystem, so this fixes it too.

WHY

Closes #922.
Since v3.15.0, _ls_dirs() caches listings under a tuple key, but invalidate_cache(path) only popped string keys.
As a result, ls() and find() kept returning stale listings until ls(refresh=True), invalidate_cache() with no argument, or a separate filesystem instance (for example with skip_instance_cache=True; fsspec otherwise returns the cached instance and its listings).
The prefix mix-up comes from the same cache key and was found while investigating the issue.
The empty-refresh, withdirs, find(refresh=True), exists(refresh=True) and not-found HeadObject/HeadBucket problems are other pre-existing cache defects, found in the independent reviews.
The maintainer agreed to fix all of them in this PR.
Other pre-existing filesystem cache and compatibility issues found in the review are filed separately as #931, #932 and #933.

TEST

Tested commit: b16c58f (rebased onto master 9b74767, which adds #919's docstrings; there were no conflicts).

• just format and just lint (including the pydocstyle rules from Document every public API in pyathena/ and check docstrings with ruff #919): passed.
• Offline: uv run pytest --noconftest tests/pyathena/filesystem/test_s3.py -k "invalidate_cache_drops or partial_listing or ls_from_cache or empty_refresh or withdirs_does_not or refresh_bypasses or refresh_evicts or missing_bucket" with dummy env: 10 passed.
  Without the corresponding s3.py changes, each of the 9 new unit tests fails. Each was checked against the commit that preceded its fix.
• Live S3: uv run --env-file .env pytest -n 1 tests/pyathena/filesystem/test_s3.py tests/pyathena/filesystem/test_s3_async.py -k "reflect_changes or invalidate or ls or partial_listing or empty_refresh or refresh or find or rm or touch or exists or info or glob or mkdir or bucket": 64 passed.
  Without the invalidate_cache change, the new test_ls_and_find_reflect_changes_through_the_filesystem fails because ls() still lists the deleted object.
• Not run locally: the rest of the filesystem tests and the full just test pyathena. AWS CI covers these once the PR is Ready.

🤖 Generated with Claude Code

[laughingman7743]

Self-review round 1 (behavior and implementation): FINDINGS (1, repaired)

Scope: base 775874c, head 6e60732 (full diff: pyathena/filesystem/s3.py, tests/pyathena/filesystem/test_s3.py).

Covered:

• Cache key normalization: ls() passes _strip_protocol(path).rstrip("/"), and _find() passes _strip_protocol(path) (fsspec strips the trailing slash), so invalidate_cache() builds the same path string for both "/" (ls, maxdepth find) and "" (recursive find) entries. Recursive _find() subcalls use s3://bucket/{item.key} with no trailing slash.
• Write paths: rm/_rm, touch, cp_file, pipe_file, put_file, setxattr, S3File.commit, bucket mkdir/rmdir, and the async _rm/_cp_file all call invalidate_cache(). put_tags/chmod do not change listing fields.
• Async: S3FileSystemAsync shares the sync dircache and delegates _ls, _find (including the prefix kwarg) and invalidate_cache to the sync instance.
• use_cache is computed before prefix is merged with key; prefixed and next_token listings neither read nor write the cache. The unprefixed recursive subcalls of find(maxdepth=...) still cache. No caller passes next_token/max_keys today.
• Tests: the 3 new unit tests and the integration test fail without the s3.py change (verified locally).

Finding (repaired in 2fba19f): test_invalidate_cache_drops_listings_of_path_and_parents asserted that a descendant listing ("bucket/a/b/c.txt/d", "") is kept. fsspec documents invalidate_cache(path) as "listings at or under given path". This PR keeps the existing path-and-parents walk (pre-v3.15.0 and s3fs behavior), so the test should not pin descendant retention. The assertion was removed. The offline unit tests and just lint were rerun and pass.

Deferred (pre-existing, out of scope): invalidate_cache(path) does not drop listings under path. Doing so would require scanning every dircache key on each write.

[laughingman7743]

Self-review round 2 (claims, callers, operations): FINDINGS (2, repaired)

Scope: base 775874c, head 2fba19f (full claim audit: PR body, commit messages, new docstrings/comment, issue #922 premises).

Claims checked:

• "tuple key since v3.15.0": git tag --contains 681e7496 gives v3.15.0 first. Holds.
• "rm/touch/pipe_file/S3File.commit/mv/cp and other writes call invalidate_cache": verified at their call sites; mv is fsspec copy + rm, and the async _rm/_cp_file call the sync invalidate_cache. Holds.
• "find(d) then find(d, prefix=x) returned the unfiltered listing": before the fix both calls use key (d, ""). The reverse order also returned the filtered listing for find(d). Holds.
• "S3FileSystemAsync shares dircache and delegates": s3_async.py:105,114,299,394. Holds.
• "invalidate_cache docstring matches Document every public API in pyathena/ and check docstrings with ruff #919": text compared, and git merge-tree of this head with Document every public API in pyathena/ and check docstrings with ruff #919 head 6be315e merges without conflict. Holds.
• DirCache.pop() on missing or expired tuple keys: MutableMapping.pop handles the KeyError from DirCache.__getitem__. No new failure mode.
• Docs: docs/filesystem.md, docs/s3fs.md and docs/aio.md make no listing-cache or prefix statements made obsolete by this change.
• Operational: prefixed find() now issues ListObjectsV2 each time (correctness over reuse), and writes now really drop listings, so the next ls() lists from S3. Both are expected. invalidate_cache adds two dict pops per path level.

Findings (repaired):

1. PR WHY said stale listings lasted until invalidate_cache() "or a new instance was created". This omitted ls(refresh=True), and constructing S3FileSystem(...) again with the same arguments returns fsspec's cached instance and its listings. PR body corrected.
2. This docstring said invalidate_cache drops the listing "for the path and its parents", which reads as if invalidating the listed path's parents drops it. Reworded in 7d84691: it is dropped when the path or a path under it is invalidated.

Evidence limits: the live S3 runs (26 sync + 17 async filesystem tests) are on 6e60732. Later commits change only the unit test assertions and docstrings; just lint and the offline unit tests were rerun on them. The full suite runs in AWS CI after Ready.

[laughingman7743]

Independent review (relayed): CLEAN for this diff; 5 pre-existing issues reported.

Reviewer: Codex CLI 0.160.0 (codex exec -s read-only, model reported as gpt-6-astra), a different model from the author (Claude). Static review: source inspection only, with no tests, builds, edits, network, GitHub or PR-discussion access. The prompt contained no PR number, description, commit messages or prior findings.
Scope: base 775874c, head 7d84691 (detached snapshot plus literal diff). The snapshot and the PR worktree were unchanged afterwards.

Covered (reviewer): _ls_dirs/ls/find cache keys, pagination, prefix, maxdepth, withdirs, refresh and path normalization; info/exists/_ls_from_cache, version ids and invalidation; sync and async writes (delete, copy, upload, touch, bucket ops, metadata/ACL/tags, buffered commit); new tests, docstrings and comments.

Result: "no actionable regression introduced by this diff". All new tests would fail against the original implementation; the integration test verifies observable listing results.

Pre-existing issues reported, each verified by the author against base 775874c:

1. s3.py:715 (_find): find(d, withdirs=True) extends the list object returned from the cache in place, so repeating the call duplicates the derived directories. Confirmed by reading the code.
2. s3.py:449 (_ls_dirs): an empty refresh=True listing neither replaces nor evicts the old entry, so the next plain ls() returns the deleted objects again. Confirmed. This also qualifies the new docstring wording "A complete listing of the path is cached".
3. s3.py:1790 (invalidate_cache): rm_file("b/d/key?versionId=v1") invalidates the version-qualified path and its ancestors but not the cached "b/d/key" HeadObject entry, so exists("b/d/key") can stay True. Confirmed.
4. s3.py:595 (info): info(p, version_id="v1") followed by info(p, version_id="v2") returns v1's cached metadata, because neither the cache lookup nor _head_object compares the version. Confirmed.
5. s3.py:696 (_find): maxdepth is off by one compared with fsspec 2026.9.0 (walk requires maxdepth >= 1, and maxdepth=1 means only the direct entries). Here maxdepth=0 lists the direct entries and maxdepth=1 descends one more level. test_find_maxdepth encodes the current behavior. Confirmed.

Disposition: pending the maintainer's decision on folding versus separate issues; follow-ups will be recorded in this thread.

[laughingman7743]

Repair 81e2310 (findings 1 and 2) and its self-review

Maintainer decision: fold findings 1 and 2 (listing cache) into this PR. Findings 3 to 5 are filed as #931 (stale HeadObject entry after deleting a version), #932 (info(version_id=...) ignores the version in the cache) and #933 (maxdepth off by one compared with fsspec).

Repair (7d84691..81e2310):

• _find() builds a new list (files + self._extract_parent_directories(...)) instead of files.extend(...) on the list that _ls_dirs() may have returned from the cache.
• _ls_dirs(): for a cacheable listing (no prefix/next_token), a non-empty result is stored and an empty result pops (path, delimiter). The docstring now says this.
• Tests: test_ls_dirs_empty_refresh_evicts_cached_listing and test_find_withdirs_does_not_modify_cached_listing (offline). Both fail on 7d84691 and pass on 81e2310.

Self-review of the repair:

• Behavior: the empty-result eviction runs only for cacheable listings, so prefixed find() calls never evict the unprefixed listing. ls(file_path) lists file_path/ (empty), so it pops a key that never holds an entry, then falls back to _head_object as before. Other cached-list consumers (ls returns list(files), find(maxdepth=...) builds result, find builds new lists or dicts) do not mutate the cache. _ls_buckets() returns its cached list, but ls() copies it.
• Claims: the commit message and the docstring ("A complete, non-empty listing ... is cached ..., and an empty one evicts it") match the code. The PR body WHAT/WHY/TEST were updated; TEST now states commit 81e2310.
• Validation: just lint passes; offline unit tests 6 passed; live test_s3.py + test_s3_async.py (-k "reflect_changes or invalidate or ls or partial_listing or empty_refresh or find or rm or touch or exists or info or glob") 55 passed on the 81e2310 tree.

An independent follow-up on this range is next.

[laughingman7743]

Independent follow-up (relayed) on 7d84691..81e2310: CLEAN; 2 more pre-existing issues

Reviewer: Codex CLI 0.160.0 (codex exec -s read-only, model reported as gpt-6-astra, session 01a0ffd0-7964-7343-a710-deb9db9c117b). Static review of the follow-up diff with the full diff as context; the snapshot at 81e2310 and the PR worktree were unchanged afterwards.
Covered: _ls_dirs() cache hits, pagination, refresh, both delimiters and the prefix/token bypass; the ls() file fallback, both _find() branches and async delegation; the two new tests (they fail on the old code); the changed docstring. Result: both targeted defects are fixed, no introduced regression, and no remaining caller mutates the cached list.
Pre-existing issues reported (both verified by the author):

• A, s3.py:337 _head_object: the not-found branch leaves the cached entry, so after ls(p, refresh=True) returns [] for an externally deleted file, the next ls(p) returns it again. _head_bucket has the same pattern.
• B, s3.py:710 _find: refresh is not forwarded, so find(d, refresh=True) returns the cached listing.

Repair 2 (maintainer: fix both in this PR), now on head c982550 after rebasing onto master 9b74767 (#919). The rebase had no conflicts; upstream only adds docstrings to pyathena/filesystem (549 insertions, 0 deletions).

• _head_object()/_head_bucket() pop the path/bucket entry when HeadObject/HeadBucket raises FileNotFoundError.
• _find() reads refresh from kwargs (kept there so the recursive maxdepth calls inherit it) and forwards it to both _ls_dirs() calls and to the info() fallback.
• find() docstring documents prefix and refresh. The tests gain a _file_object() helper for the new unit tests.
• Tests: test_find_refresh_bypasses_cached_listings (recursive and maxdepth=1, including a stale subdirectory listing) and test_refresh_evicts_cached_object_and_bucket_not_found. Both fail on 81e2310 and pass now.

Self-review of repair 2:

• Behavior: the pop runs only after a real HEAD returned not found, so uncached misses are no-ops. With an explicit version_id whose HEAD is not found, the unversioned entry is evicted, which costs only a later HEAD. PermissionError and other errors do not evict. refresh is now honored by glob(), which calls find() with the caller's kwargs. The async _find forwards kwargs to the sync _find, so it inherits the fix.
• Claims: the commit messages and the find() docstring match the code. The PR title and body were updated (WHAT/WHY/TEST, tested commit c982550).
• Deferred (pre-existing, from Document every public API in pyathena/ and check docstrings with ruff #919): the new info() docstring says a cached listing of the path or its parent decides directory or not-found. With tuple-keyed listings, that applies only to the bucket list under "". This belongs to the docs/docstring audit Check the user guides and docstrings against the implementation #927, outside this diff.
• Validation: just lint (with the pydocstyle rules) passes; offline unit tests 8 passed; live test_s3.py + test_s3_async.py filtered run 57 passed on c982550.

An independent follow-up on this range is next.

[laughingman7743]

Independent follow-up 2 (relayed) on 38a2e28..c982550 (rebased series): FINDINGS (1, P3); 3 pre-existing issues

Reviewer: Codex CLI 0.160.0 (codex exec -s read-only, model reported as gpt-6-astra, session 01a0ffdc-1cea-7c52-8017-c4c6273e0bd7). Static review with the range-diff (775874c..81e2310 vs 9b74767..c982550), the follow-up diff and the full diff. The snapshot at c982550 and the PR worktree were unchanged afterwards.
Covered: _head_object, _head_bucket, cache lookup, info, exists, ls, both find paths, inherited sync/async glob, and the async wrappers. Results: A evicts the direct entries on FileNotFoundError and keeps them on other errors. B reaches both delimiters, every recursive level and the info() fallback. The rebase contains only the expected docstring merge. The new tests fail on the old code and assert observable results.

• Finding (P3, introduced): the new find() prefix doc claimed unconditional filtering, but find("s3://bucket/file", prefix="unmatched") returns the object through the info() fallback.
• Pre-existing: (1) after ls("s3://"), a deleted bucket comes back from the cached bucket listing dircache[""] even after info(bucket, refresh=True); (2) exists(path, refresh=True) ignores refresh; (3) a keyword version_id shares the path cache key. (3) is S3FileSystem.info(version_id=...) returns cached metadata of another version #932.

Repair 3 (a95e84f): the docstring now says that if nothing is listed and the path itself is an object, it is returned regardless of the prefix. _head_bucket() also pops dircache[""] on not found, completing A, as mkdir()/rmdir() already do for buckets. test_refresh_evicts_cached_object_and_bucket_not_found now seeds the bucket listing too, and fails without the change.
Repair 4 (51a4362, maintainer: fix (2) in this PR): exists() pops refresh. With refresh=True it skips the _ls_from_cache/dircache checks and passes refresh to info() (keys) and _head_bucket() (buckets). Without refresh, the logic is unchanged. Docstring updated. New test_exists_refresh_bypasses_cache fails without the change.

Self-review of repairs 3 and 4:

• Behavior: internal callers (mkdir, etc.) call exists() without refresh, so their path is unchanged. With refresh=True on a key prefix, info() falls through to the MaxKeys=1 listing and returns True. Popping "" on a not-found bucket costs one later ListBuckets. The async _exists forwards kwargs to the sync exists.
• Claims: the commit messages, the exists()/find() docstrings and the updated PR body (tested commit 51a4362) match the code.
• Validation: just lint passes; offline unit tests 9 passed (plus the offline mkdir/rmdir unit tests); live test_s3.py + test_s3_async.py filtered run (adds mkdir or bucket) 63 passed on 51a4362.

An independent follow-up on a95e84f and 51a4362 is next.

[laughingman7743]

Independent follow-up 3 (relayed) on c982550..51a4362: FINDINGS (2, both introduced by repairs 3/4)

Reviewer: Codex CLI 0.160.0 (codex exec -s read-only, model reported as gpt-6-astra, session 01a0ffe9-3cbc-79b3-9403-2847a5d46510). Static review with the follow-up diff and full diff 9b74767..51a4362. The snapshot at 51a4362 and the PR worktree were unchanged afterwards.
Covered: find()/_find(), info(), the object/bucket HEAD paths, cache lookup and eviction; exists() with and without refresh; internal callers (mkdir, makedirs, rmdir, touch, pipe_file, append); async _exists(). Refresh forwarding works, the stale deleted-bucket lookup is repaired, and the tests assert observable behavior and fail on the previous code.

1. P2, s3.py:320: popping dircache[""] on any missing bucket also drops a listing that never contained it. After ls("s3://"), exists("s3://missing") makes later exists() of listed buckets issue HeadBucket, which raises PermissionError for callers with s3:ListAllMyBuckets but no s3:ListBucket. The same happens through mkdir("s3://missing/prefix", create_parents=False).
2. P3, s3.py:796: the new prefix fallback note does not hold with maxdepth; the depth-limited branch never falls back to info(path).

Repair 5 (b16c58f): _head_bucket() evicts the bucket listing only if it lists the missing bucket (b.name == bucket; bucket S3Object names are the bucket name). The prefix note now starts with "Without maxdepth". New test_missing_bucket_keeps_bucket_listing_without_it fails on 51a4362. test_refresh_evicts_cached_object_and_bucket_not_found (listing contains the bucket) still passes.
Self-review: only _ls_buckets() writes dircache[""], always as S3Object buckets, so b.name is safe. Behavior without a missing bucket is unchanged. The commit message, docstring and PR body (tested commit b16c58f) match. just lint passes; offline unit tests 10 passed; live filtered test_s3.py + test_s3_async.py run 64 passed on b16c58f.

An independent follow-up on b16c58f is next.

[laughingman7743]

Independent follow-up 4 (relayed) on 51a4362..b16c58f: CLEAN

Reviewer: Codex CLI 0.160.0 (codex exec -s read-only, model reported as gpt-6-astra, session 01a0fff1-e2f7-7183-bb91-e5968b561c7d). Static review (no edits, builds, tests, network or GitHub) of the follow-up diff, with the full diff 9b74767..b16c58f as context. The snapshot at b16c58f and the PR worktree were unchanged afterwards.
Covered: _head_bucket(), info(), exists(), mkdir(), rmdir() and the async wrappers. Refresh reaches HeadBucket. A missing bucket loses its direct cache entry and any bucket listing that contains it, so later lookups cannot return it from the cache. _ls_buckets() is the only writer of dircache[""] and stores list[S3Object]; absent or empty entries are safe. Successful bucket creation and deletion still evict the listing. The new test fails on the previous code at assert fs.exists("s3://bucket"). The changed docstring is accurate: the info(path) fallback only runs when maxdepth is None.
Result: no actionable findings, and no further pre-existing issues reported.

This completes the independent review for head b16c58f. Next: confirm the offline checks and mergeability, then mark the PR Ready for AWS CI.