From 08a8e54d87d2a8f9d0508fdbb74fddb5bea0672d Mon Sep 17 00:00:00 2001 From: Will Miao Date: Wed, 7 Oct 2026 16:35:29 +0800 Subject: [PATCH] fix(config): keep unavailable model roots in the saved library paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Starting ComfyUI with an external drive switched off erased that drive's path from `settings.json`. `Config._dedupe_existing_paths()` drops paths that do not exist at the moment the root list is built, and `save_folder_paths_to_settings()` (plugin mode, called from `Config.__init__`) persisted `list(self.loras_roots)` through `upsert_library(folder_paths=...)`, which REPLACES the library's paths. One boot with the drive off was therefore enough to lose the configuration, not just to hide the root — the same applied to checkpoints/unet/embeddings and the other-model keys. * `Config` now records the *configured* (existence-unfiltered) primary paths per model type while building the live lists, and the startup sync persists those. A path the host (plugin mode) or the library (standalone mode) no longer configures is still dropped, so removing a path keeps working. * In standalone mode the host mock (`MockFolderPaths.get_folder_paths`) already filters non-existent paths out of settings.json, so the active library snapshot is the only record of what the user configured; `_remember_library_configured_paths()` reads it there and leaves plugin mode to ComfyUI's own unfiltered list. * `_resolve_valid_default_root()` receives the configured paths in `allowed_paths`, so a `default_*_root` on a switched-off drive is no longer "repaired" onto another root. * `_append_new_paths()` is the shared append-only union (order preserved, case insensitive) used by the sync. Verified: a new config test builds the plugin-mode sync with one switched-off drive, one readable root and one path the host dropped — the switched-off path and the default root that points at it survive, the dropped path does not, and the live `loras_roots` still holds only the readable root. tests/config 64 passed. --- py/config.py | 188 ++++++++++++++++++++++++- tests/config/test_config_save_paths.py | 63 +++++++++ 2 files changed, 244 insertions(+), 7 deletions(-) diff --git a/py/config.py b/py/config.py index 20f9e1a2..5a0d2204 100644 --- a/py/config.py +++ b/py/config.py @@ -44,6 +44,26 @@ def _normalize_root_identity(path: str) -> str: return normalized +def _append_new_paths(current: List[str], candidates: Iterable[str]) -> List[str]: + """Return ``current`` plus any candidate it does not already hold. + + Order is preserved and nothing is ever removed: ``*_roots[0]`` derives the + recipes directory and the usage-stats file location, so a mid-session change + to the root *order* would move user data. + """ + merged = list(current) + seen = {_normalize_root_identity(path) for path in merged} + for path in candidates: + if not isinstance(path, str) or not path.strip(): + continue + identity = _normalize_root_identity(path) + if identity in seen: + continue + seen.add(identity) + merged.append(path) + return merged + + def _resolve_valid_default_root( current: str, primary_paths: List[str], allowed_paths: List[str], name: str ) -> str: @@ -169,6 +189,12 @@ class Config: self._preview_root_paths: Set[Path] = set() # Fingerprint of the symlink layout from the last successful scan self._cached_fingerprint: Optional[Dict[str, object]] = None + # Configured (existence-unfiltered) primary roots per model type. The live + # lists below drop paths whose directory is missing right now (a drive + # that is switched off), which must not be mistaken for "the user + # removed this path": these are what `save_folder_paths_to_settings()` + # persists and what `/roots` reports as unavailable. + self._configured_root_paths: Dict[str, List[str]] = {} self.loras_roots = self._init_lora_paths() self.checkpoints_roots = None self.unet_roots = None @@ -228,6 +254,8 @@ class Config: if isinstance(recipes_path, str) and recipes_path: self.recipes_path = recipes_path + self._remember_library_configured_paths(library_config) + extra_folder_paths = library_config.get("extra_folder_paths") if not isinstance(extra_folder_paths, dict): return @@ -340,16 +368,40 @@ class Config: comfy_library = libraries.get("comfyui", {}) default_library = libraries.get("default", {}) + # Persist what is *configured*, not what is readable right now: + # `upsert_library(folder_paths=...)` replaces the library's paths, so + # writing the existence-filtered live lists would erase the path of a + # drive that happened to be switched off when ComfyUI started. target_folder_paths = { - "loras": list(self.loras_roots), - "checkpoints": list(self.checkpoints_roots or []), - "unet": list(self.unet_roots or []), - "embeddings": list(self.embeddings_roots or []), + "loras": _append_new_paths( + self.configured_roots_for("lora"), self.loras_roots or [] + ), + "checkpoints": _append_new_paths( + self.configured_roots_for("checkpoint"), + self.checkpoints_roots or [], + ), + "unet": _append_new_paths( + self.configured_roots_for("unet"), self.unet_roots or [] + ), + "embeddings": _append_new_paths( + self.configured_roots_for("embedding"), + self.embeddings_roots or [], + ), } # Persist the other-model roots under their original folder_paths # keys so library switching round-trips them. for key, roots in (self.other_folder_roots or {}).items(): target_folder_paths[key] = list(roots) + # ...and keep an other-model root that is configured but unavailable. + configured_other = self.configured_roots_for("other") + if configured_other: + for key in self._get_enabled_other_folder_keys(): + configured_key_roots = self._configured_other_paths_for_key(key) + if not configured_key_roots: + continue + target_folder_paths[key] = _append_new_paths( + configured_key_roots, target_folder_paths.get(key, []) + ) normalized_target_paths = _normalize_folder_paths_for_comparison( target_folder_paths @@ -420,7 +472,14 @@ class Config: default_lora_root = _resolve_valid_default_root( comfy_library.get("default_lora_root", ""), - list(self.loras_roots or []), + # A configured-but-unavailable root is still a valid choice: it + # must not be "repaired" away just because its drive is off. + _append_new_paths( + self.configured_roots_for( + "lora" + ), + self.loras_roots or [], + ), list(self.loras_roots or []) + list(comfy_library.get("extra_folder_paths", {}).get("loras", []) or []), "default_lora_root", @@ -428,7 +487,14 @@ class Config: default_checkpoint_root = _resolve_valid_default_root( comfy_library.get("default_checkpoint_root", ""), - list(self.checkpoints_roots or []), + # A configured-but-unavailable root is still a valid choice: it + # must not be "repaired" away just because its drive is off. + _append_new_paths( + self.configured_roots_for( + "checkpoint" + ), + self.checkpoints_roots or [], + ), list(self.checkpoints_roots or []) + list(comfy_library.get("extra_folder_paths", {}).get("checkpoints", []) or []), "default_checkpoint_root", @@ -436,7 +502,14 @@ class Config: default_embedding_root = _resolve_valid_default_root( comfy_library.get("default_embedding_root", ""), - list(self.embeddings_roots or []), + # A configured-but-unavailable root is still a valid choice: it + # must not be "repaired" away just because its drive is off. + _append_new_paths( + self.configured_roots_for( + "embedding" + ), + self.embeddings_roots or [], + ), list(self.embeddings_roots or []) + list(comfy_library.get("extra_folder_paths", {}).get("embeddings", []) or []), "default_embedding_root", @@ -1331,6 +1404,14 @@ class Config: unet_paths = folder_paths.get("unet", []) or [] embedding_paths = folder_paths.get("embeddings", []) or [] + # The snapshot is the authoritative configured set: unlike the live lists + # below it keeps paths whose directory is missing right now, and a path + # the user removed from the library is forgotten here. + self._remember_configured_paths("lora", lora_paths) + self._remember_configured_paths("checkpoint", checkpoint_paths) + self._remember_configured_paths("unet", unet_paths) + self._remember_configured_paths("embedding", embedding_paths) + self.loras_roots = self._prepare_lora_paths(lora_paths) ( self.base_models_roots, @@ -1343,6 +1424,10 @@ class Config: key: folder_paths.get(key, []) or [] for key in self._get_enabled_other_folder_keys() } + self._remember_configured_paths( + "other", + [path for paths in other_path_map.values() for path in paths], + ) ( self.other_roots, self.other_root_subtypes, @@ -1398,10 +1483,60 @@ class Config: self._initialize_symlink_mappings() + def _remember_library_configured_paths( + self, library_config: Mapping[str, Any] + ) -> None: + """Remember the paths the library configures, missing directories included. + + In standalone mode the host mock already filters non-existent paths out of + ``folder_paths`` (see ``standalone.MockFolderPaths.get_folder_paths``), so + the library snapshot is the only record of what the user configured and it + wins there. In plugin mode ComfyUI's own list is unfiltered and therefore + authoritative — a path removed from the host config must be forgotten, not + resurrected from our mirror of it. + """ + if not standalone_mode: + return + + from .services.settings_manager import get_settings_manager + + folder_paths_map = ( + library_config.get("folder_paths") + if isinstance(library_config, Mapping) + else None + ) + if not isinstance(folder_paths_map, Mapping): + folder_paths_map = getattr( + get_settings_manager(), "settings", {} + ).get("folder_paths", {}) + if not isinstance(folder_paths_map, Mapping): + return + + for key, model_type in ( + ("loras", "lora"), + ("checkpoints", "checkpoint"), + ("unet", "unet"), + ("embeddings", "embedding"), + ): + paths = folder_paths_map.get(key) + if isinstance(paths, (list, tuple)): + self._remember_configured_paths(model_type, paths) + + core_keys = {"loras", "checkpoints", "unet", "embeddings"} + other_paths = [ + path + for key, paths in folder_paths_map.items() + if key not in core_keys and isinstance(paths, (list, tuple)) + for path in paths + ] + if other_paths: + self._remember_configured_paths("other", other_paths) + def _init_lora_paths(self) -> List[str]: """Initialize and validate LoRA paths from ComfyUI settings""" try: raw_paths = folder_paths.get_folder_paths("loras") + self._remember_configured_paths("lora", raw_paths) unique_paths = self._prepare_lora_paths(raw_paths) logger.info( "Found LoRA roots:" @@ -1422,6 +1557,8 @@ class Config: try: raw_checkpoint_paths = folder_paths.get_folder_paths("checkpoints") raw_unet_paths = folder_paths.get_folder_paths("unet") + self._remember_configured_paths("checkpoint", raw_checkpoint_paths) + self._remember_configured_paths("unet", raw_unet_paths) ( unique_paths, self.checkpoints_roots, @@ -1448,6 +1585,7 @@ class Config: """Initialize and validate embedding paths from ComfyUI settings""" try: raw_paths = folder_paths.get_folder_paths("embeddings") + self._remember_configured_paths("embedding", raw_paths) unique_paths = self._prepare_embedding_paths(raw_paths) logger.info( "Found embedding roots:" @@ -1485,6 +1623,10 @@ class Config: except Exception as exc: logger.debug("Error reading folder paths for '%s': %s", key, exc) + self._remember_configured_paths( + "other", [path for paths in folder_path_map.values() for path in paths] + ) + ( unique_paths, self.other_root_subtypes, @@ -1505,6 +1647,38 @@ class Config: logger.warning(f"Error initializing other model paths: {e}") return [] + def _remember_configured_paths( + self, model_type: str, paths: Iterable[str] + ) -> None: + """Record the configured roots of a model type (they may not exist yet). + + Replaces the previous value rather than merging: every caller knows the + complete configured set for its type, so a path the user removed must be + forgotten here too. + """ + configured = [ + path.strip() + for path in paths or [] + if isinstance(path, str) and path.strip() + ] + # Tolerate partially constructed instances (`Config.__new__`), which the + # path-resolution tests build to exercise a single initializer. + configured_map = getattr(self, "_configured_root_paths", None) + if configured_map is None: + configured_map = {} + self._configured_root_paths = configured_map + configured_map[model_type] = configured + + def configured_roots_for(self, model_type: str) -> List[str]: + """Configured roots of a model type, including ones that are unavailable. + + ``get_model_roots()`` on the scanners answers "what can be walked right + now"; this answers "what did the user (or the host) configure", which is + the set that must survive a switched-off drive in the configuration and + the set the refresh menu reports as offline. + """ + return list((getattr(self, "_configured_root_paths", None) or {}).get(model_type, [])) + def refresh_other_roots(self) -> None: """Rebuild other-model roots after the management toggles changed. diff --git a/tests/config/test_config_save_paths.py b/tests/config/test_config_save_paths.py index 5e4db62c..599c3099 100644 --- a/tests/config/test_config_save_paths.py +++ b/tests/config/test_config_save_paths.py @@ -905,3 +905,66 @@ def test_save_paths_removes_stale_empty_default_when_comfyui_exists( assert name == "comfyui" assert payload["activate"] is True assert fake_settings.active_library == "comfyui" + + +def test_save_paths_keeps_configured_root_of_a_switched_off_drive( + monkeypatch: pytest.MonkeyPatch, tmp_path +): + """A root whose directory is missing right now must stay in the library. + + `upsert_library(folder_paths=...)` replaces the stored paths, so persisting + the existence-filtered live list would erase a drive that happened to be + switched off when ComfyUI started. A path the host no longer configures must + still be dropped. + """ + folder_paths = _setup_config_environment(monkeypatch, tmp_path) + readable = folder_paths["loras"][0] + switched_off = str(tmp_path / "drive-Z" / "loras") + removed_by_user = str(tmp_path / "old-location" / "loras") + # The host still configures the switched-off drive; it no longer configures + # the old location (that one only survives in the stored library paths). + folder_paths["loras"] = [switched_off, readable] + + class FakeSettingsService: + active_library = "comfyui" + name: str = "" + payload: Dict[str, Any] = {} + + def get_libraries(self): + return { + "comfyui": { + "folder_paths": { + "loras": [removed_by_user, readable], + "checkpoints": [], + "unet": [], + "embeddings": [], + }, + "default_lora_root": switched_off, + } + } + + def rename_library(self, *_): + raise AssertionError("rename_library should not be invoked") + + def get_active_library_name(self): + return self.active_library + + def upsert_library(self, name: str, **payload): + self.name = name + self.payload = payload + + fake_settings = FakeSettingsService() + monkeypatch.setattr(settings_manager_module, "settings", fake_settings) + + config_instance = config_module.Config() + + persisted = fake_settings.payload["folder_paths"]["loras"] + # The switched-off drive keeps its configured path... + assert switched_off.replace("\\", "/") in persisted + assert readable.replace("\\", "/") in persisted + # ...the path the host really dropped is gone... + assert removed_by_user.replace("\\", "/") not in persisted + # ...and the live list still only holds what is readable right now. + assert config_instance.loras_roots == [readable.replace("\\", "/")] + # The user's default root survives because it is merely unavailable. + assert fake_settings.payload["default_lora_root"] == switched_off.replace("\\", "/")