mirror of
https://github.com/willmiao/ComfyUI-Lora-Manager.git
synced 2026-10-07 18:12:12 -03:00
fix(config): keep unavailable model roots in the saved library paths
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.
This commit is contained in:
+181
-7
@@ -44,6 +44,26 @@ def _normalize_root_identity(path: str) -> str:
|
|||||||
return normalized
|
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(
|
def _resolve_valid_default_root(
|
||||||
current: str, primary_paths: List[str], allowed_paths: List[str], name: str
|
current: str, primary_paths: List[str], allowed_paths: List[str], name: str
|
||||||
) -> str:
|
) -> str:
|
||||||
@@ -169,6 +189,12 @@ class Config:
|
|||||||
self._preview_root_paths: Set[Path] = set()
|
self._preview_root_paths: Set[Path] = set()
|
||||||
# Fingerprint of the symlink layout from the last successful scan
|
# Fingerprint of the symlink layout from the last successful scan
|
||||||
self._cached_fingerprint: Optional[Dict[str, object]] = None
|
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.loras_roots = self._init_lora_paths()
|
||||||
self.checkpoints_roots = None
|
self.checkpoints_roots = None
|
||||||
self.unet_roots = None
|
self.unet_roots = None
|
||||||
@@ -228,6 +254,8 @@ class Config:
|
|||||||
if isinstance(recipes_path, str) and recipes_path:
|
if isinstance(recipes_path, str) and recipes_path:
|
||||||
self.recipes_path = recipes_path
|
self.recipes_path = recipes_path
|
||||||
|
|
||||||
|
self._remember_library_configured_paths(library_config)
|
||||||
|
|
||||||
extra_folder_paths = library_config.get("extra_folder_paths")
|
extra_folder_paths = library_config.get("extra_folder_paths")
|
||||||
if not isinstance(extra_folder_paths, dict):
|
if not isinstance(extra_folder_paths, dict):
|
||||||
return
|
return
|
||||||
@@ -340,16 +368,40 @@ class Config:
|
|||||||
comfy_library = libraries.get("comfyui", {})
|
comfy_library = libraries.get("comfyui", {})
|
||||||
default_library = libraries.get("default", {})
|
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 = {
|
target_folder_paths = {
|
||||||
"loras": list(self.loras_roots),
|
"loras": _append_new_paths(
|
||||||
"checkpoints": list(self.checkpoints_roots or []),
|
self.configured_roots_for("lora"), self.loras_roots or []
|
||||||
"unet": list(self.unet_roots or []),
|
),
|
||||||
"embeddings": list(self.embeddings_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
|
# Persist the other-model roots under their original folder_paths
|
||||||
# keys so library switching round-trips them.
|
# keys so library switching round-trips them.
|
||||||
for key, roots in (self.other_folder_roots or {}).items():
|
for key, roots in (self.other_folder_roots or {}).items():
|
||||||
target_folder_paths[key] = list(roots)
|
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(
|
normalized_target_paths = _normalize_folder_paths_for_comparison(
|
||||||
target_folder_paths
|
target_folder_paths
|
||||||
@@ -420,7 +472,14 @@ class Config:
|
|||||||
|
|
||||||
default_lora_root = _resolve_valid_default_root(
|
default_lora_root = _resolve_valid_default_root(
|
||||||
comfy_library.get("default_lora_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(self.loras_roots or [])
|
||||||
+ list(comfy_library.get("extra_folder_paths", {}).get("loras", []) or []),
|
+ list(comfy_library.get("extra_folder_paths", {}).get("loras", []) or []),
|
||||||
"default_lora_root",
|
"default_lora_root",
|
||||||
@@ -428,7 +487,14 @@ class Config:
|
|||||||
|
|
||||||
default_checkpoint_root = _resolve_valid_default_root(
|
default_checkpoint_root = _resolve_valid_default_root(
|
||||||
comfy_library.get("default_checkpoint_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(self.checkpoints_roots or [])
|
||||||
+ list(comfy_library.get("extra_folder_paths", {}).get("checkpoints", []) or []),
|
+ list(comfy_library.get("extra_folder_paths", {}).get("checkpoints", []) or []),
|
||||||
"default_checkpoint_root",
|
"default_checkpoint_root",
|
||||||
@@ -436,7 +502,14 @@ class Config:
|
|||||||
|
|
||||||
default_embedding_root = _resolve_valid_default_root(
|
default_embedding_root = _resolve_valid_default_root(
|
||||||
comfy_library.get("default_embedding_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(self.embeddings_roots or [])
|
||||||
+ list(comfy_library.get("extra_folder_paths", {}).get("embeddings", []) or []),
|
+ list(comfy_library.get("extra_folder_paths", {}).get("embeddings", []) or []),
|
||||||
"default_embedding_root",
|
"default_embedding_root",
|
||||||
@@ -1331,6 +1404,14 @@ class Config:
|
|||||||
unet_paths = folder_paths.get("unet", []) or []
|
unet_paths = folder_paths.get("unet", []) or []
|
||||||
embedding_paths = folder_paths.get("embeddings", []) 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.loras_roots = self._prepare_lora_paths(lora_paths)
|
||||||
(
|
(
|
||||||
self.base_models_roots,
|
self.base_models_roots,
|
||||||
@@ -1343,6 +1424,10 @@ class Config:
|
|||||||
key: folder_paths.get(key, []) or []
|
key: folder_paths.get(key, []) or []
|
||||||
for key in self._get_enabled_other_folder_keys()
|
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_roots,
|
||||||
self.other_root_subtypes,
|
self.other_root_subtypes,
|
||||||
@@ -1398,10 +1483,60 @@ class Config:
|
|||||||
|
|
||||||
self._initialize_symlink_mappings()
|
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]:
|
def _init_lora_paths(self) -> List[str]:
|
||||||
"""Initialize and validate LoRA paths from ComfyUI settings"""
|
"""Initialize and validate LoRA paths from ComfyUI settings"""
|
||||||
try:
|
try:
|
||||||
raw_paths = folder_paths.get_folder_paths("loras")
|
raw_paths = folder_paths.get_folder_paths("loras")
|
||||||
|
self._remember_configured_paths("lora", raw_paths)
|
||||||
unique_paths = self._prepare_lora_paths(raw_paths)
|
unique_paths = self._prepare_lora_paths(raw_paths)
|
||||||
logger.info(
|
logger.info(
|
||||||
"Found LoRA roots:"
|
"Found LoRA roots:"
|
||||||
@@ -1422,6 +1557,8 @@ class Config:
|
|||||||
try:
|
try:
|
||||||
raw_checkpoint_paths = folder_paths.get_folder_paths("checkpoints")
|
raw_checkpoint_paths = folder_paths.get_folder_paths("checkpoints")
|
||||||
raw_unet_paths = folder_paths.get_folder_paths("unet")
|
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,
|
unique_paths,
|
||||||
self.checkpoints_roots,
|
self.checkpoints_roots,
|
||||||
@@ -1448,6 +1585,7 @@ class Config:
|
|||||||
"""Initialize and validate embedding paths from ComfyUI settings"""
|
"""Initialize and validate embedding paths from ComfyUI settings"""
|
||||||
try:
|
try:
|
||||||
raw_paths = folder_paths.get_folder_paths("embeddings")
|
raw_paths = folder_paths.get_folder_paths("embeddings")
|
||||||
|
self._remember_configured_paths("embedding", raw_paths)
|
||||||
unique_paths = self._prepare_embedding_paths(raw_paths)
|
unique_paths = self._prepare_embedding_paths(raw_paths)
|
||||||
logger.info(
|
logger.info(
|
||||||
"Found embedding roots:"
|
"Found embedding roots:"
|
||||||
@@ -1485,6 +1623,10 @@ class Config:
|
|||||||
except Exception as exc:
|
except Exception as exc:
|
||||||
logger.debug("Error reading folder paths for '%s': %s", key, 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,
|
unique_paths,
|
||||||
self.other_root_subtypes,
|
self.other_root_subtypes,
|
||||||
@@ -1505,6 +1647,38 @@ class Config:
|
|||||||
logger.warning(f"Error initializing other model paths: {e}")
|
logger.warning(f"Error initializing other model paths: {e}")
|
||||||
return []
|
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:
|
def refresh_other_roots(self) -> None:
|
||||||
"""Rebuild other-model roots after the management toggles changed.
|
"""Rebuild other-model roots after the management toggles changed.
|
||||||
|
|
||||||
|
|||||||
@@ -905,3 +905,66 @@ def test_save_paths_removes_stale_empty_default_when_comfyui_exists(
|
|||||||
assert name == "comfyui"
|
assert name == "comfyui"
|
||||||
assert payload["activate"] is True
|
assert payload["activate"] is True
|
||||||
assert fake_settings.active_library == "comfyui"
|
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("\\", "/")
|
||||||
|
|||||||
Reference in New Issue
Block a user