Fix get_filename_list crash when a cached model folder is deleted#14497
Fix get_filename_list crash when a cached model folder is deleted#14497JSap0914 wants to merge 1 commit into
Conversation
cached_filename_list_ probes os.path.getmtime() for every directory recorded while the filename cache was built, including subfolders. If one of those folders is removed at runtime (e.g. the user deletes a model folder), getmtime() raises FileNotFoundError, which propagates out of get_filename_list() and breaks model listing instead of simply rebuilding the cache. Treat an inaccessible tracked folder as a stale-cache signal and return None so the list is rebuilt.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Fixes folder_paths filename cache invalidation when a previously tracked subfolder is deleted, and adds a regression test to ensure the cache rebuilds instead of raising during mtime checks.
Changes:
- Treat missing/inaccessible tracked folders as a stale cache condition during cache validation.
- Add a unit test that deletes a tracked subfolder and verifies
get_filename_list()rebuilds correctly.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| tests-unit/folder_paths_test/cache_invalidation_test.py | Adds regression coverage for deleting a tracked subfolder after cache population. |
| folder_paths.py | Updates cache validation to handle deleted/inaccessible tracked folders without raising. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @pytest.fixture | ||
| def model_folder(): | ||
| """Register a temporary model category with a tracked subfolder.""" | ||
| folder_name = "cache_invalidation_test_cat" |
| open(os.path.join(category, "a.safetensors"), "w").close() | ||
| open(os.path.join(subfolder, "b.safetensors"), "w").close() | ||
|
|
||
| folder_paths.folder_names_and_paths[folder_name] = ( |
| folder_paths.folder_names_and_paths.pop(folder_name, None) | ||
| folder_paths.filename_list_cache.pop(folder_name, None) | ||
| folder_paths.cache_helper.clear() |
| refreshed = folder_paths.get_filename_list(folder_name) | ||
| assert refreshed == ["a.safetensors"] |
| try: | ||
| if os.path.getmtime(folder) != time_modified: | ||
| return None | ||
| except OSError: | ||
| # A tracked folder was deleted or became inaccessible; treat the | ||
| # cache as stale so it gets rebuilt instead of raising. | ||
| return None |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bug
get_filename_list()crashes withFileNotFoundErrorwhen a modelfolder that was present at cache-build time is deleted while ComfyUI is
running.
cached_filename_list_()validates the cache by comparing the storedmtime of every directory recorded during the previous scan
(
recursive_searchrecords each subfolder's mtime, not just thetop-level roots). It does this with a bare
os.path.getmtime(folder).If any tracked folder has since been removed,
getmtime()raisesFileNotFoundError, which propagates out ofget_filename_list()andbreaks model listing for that category — instead of just invalidating
the stale cache and rebuilding it.
Repro:
Fix
Treat an inaccessible tracked folder as a stale-cache signal: catch
OSErroraround thegetmtimeprobe and returnNone, so the cache isrebuilt rather than raising. This mirrors the existing behaviour when a
folder's mtime simply changed.
Verification
The suite includes a new regression test
(
tests-unit/folder_paths_test/cache_invalidation_test.py) that buildsthe cache, deletes a tracked subfolder, and asserts the next
get_filename_list()call rebuilds successfully instead of crashing. Itfails on
masterand passes with this change.