From 34f03d6495b025add3b38acf98480b42a63a4d2a Mon Sep 17 00:00:00 2001 From: Will Miao Date: Mon, 20 Apr 2026 15:37:29 +0800 Subject: [PATCH] fix(settings): preserve extra default roots in comfyui sync --- py/config.py | 77 ++++++++-- py/services/settings_manager.py | 48 +++++-- tests/config/test_config_save_paths.py | 183 ++++++++++++++++++++++++ tests/services/test_settings_manager.py | 37 +++++ 4 files changed, 324 insertions(+), 21 deletions(-) diff --git a/py/config.py b/py/config.py index 228491d7..997e76c2 100644 --- a/py/config.py +++ b/py/config.py @@ -26,20 +26,44 @@ logger = logging.getLogger(__name__) def _resolve_valid_default_root( - current: str, primary_paths: List[str], name: str + current: str, primary_paths: List[str], allowed_paths: List[str], name: str ) -> str: - """Return a valid default root from the current primary path set.""" + """Return a valid default root from the current primary/extra path set.""" valid_paths = [path for path in primary_paths if isinstance(path, str) and path.strip()] - if not valid_paths: - return "" + fallback_paths: List[str] = [] + seen: Set[str] = set() + for path in allowed_paths: + if not isinstance(path, str): + continue + stripped = path.strip() + if not stripped or stripped in seen: + continue + seen.add(stripped) + fallback_paths.append(stripped) - if current in valid_paths: + allowed = set(fallback_paths) + + if current and current in allowed: return current + if not valid_paths: + if not fallback_paths: + return "" + if current: + logger.info( + "Repaired stale %s from '%s' to '%s' because it is not present in primary or extra roots", + name, + current, + fallback_paths[0], + ) + else: + logger.info("Auto-setting %s to '%s'", name, fallback_paths[0]) + return fallback_paths[0] + if current: logger.info( - "Repaired stale %s from '%s' to '%s'", + "Repaired stale %s from '%s' to '%s' because it is not present in primary or extra roots", name, current, valid_paths[0], @@ -226,39 +250,76 @@ class Config: default_lora_root = _resolve_valid_default_root( comfy_library.get("default_lora_root", ""), list(self.loras_roots or []), + list(self.loras_roots or []) + + list(comfy_library.get("extra_folder_paths", {}).get("loras", []) or []), "default_lora_root", ) default_checkpoint_root = _resolve_valid_default_root( comfy_library.get("default_checkpoint_root", ""), list(self.checkpoints_roots or []), + list(self.checkpoints_roots or []) + + list(comfy_library.get("extra_folder_paths", {}).get("checkpoints", []) or []), "default_checkpoint_root", ) default_embedding_root = _resolve_valid_default_root( comfy_library.get("default_embedding_root", ""), list(self.embeddings_roots or []), + list(self.embeddings_roots or []) + + list(comfy_library.get("extra_folder_paths", {}).get("embeddings", []) or []), "default_embedding_root", ) metadata = dict(comfy_library.get("metadata", {})) metadata.setdefault("display_name", "ComfyUI") metadata["source"] = "comfyui" + extra_folder_paths = {} + if isinstance(comfy_library, Mapping): + existing_extra_paths = comfy_library.get("extra_folder_paths", {}) + if isinstance(existing_extra_paths, Mapping): + extra_folder_paths = { + key: list(value) if isinstance(value, list) else [] + for key, value in existing_extra_paths.items() + } + + active_library_name = settings_service.get_active_library_name() + should_activate = ( + active_library_name == "comfyui" + or self._should_activate_comfy_library(libraries, libraries_changed) + ) settings_service.upsert_library( "comfyui", folder_paths=target_folder_paths, + extra_folder_paths=extra_folder_paths, default_lora_root=default_lora_root, default_checkpoint_root=default_checkpoint_root, default_embedding_root=default_embedding_root, metadata=metadata, - activate=True, + activate=should_activate, ) - logger.info("Updated 'comfyui' library with current folder paths") + if should_activate: + logger.info("Updated 'comfyui' library with current folder paths") + else: + logger.info( + "Updated 'comfyui' library with current folder paths without activating it" + ) except Exception as e: logger.warning(f"Failed to save folder paths: {e}") + def _should_activate_comfy_library( + self, libraries: Mapping[str, Any], libraries_changed: bool + ) -> bool: + """Return whether startup sync should make the ComfyUI library active.""" + + if libraries_changed: + return True + if not libraries: + return True + return "comfyui" in libraries and len(libraries) == 1 + def _is_link(self, path: str) -> bool: try: if os.path.islink(path): diff --git a/py/services/settings_manager.py b/py/services/settings_manager.py index 08279982..2c1fc727 100644 --- a/py/services/settings_manager.py +++ b/py/services/settings_manager.py @@ -763,34 +763,29 @@ class SettingsManager: if self._preserve_disk_template: return - folder_paths = self.settings.get("folder_paths", {}) updated = False def _check_and_auto_set(key: str, setting_key: str) -> bool: """Repair default roots when empty or no longer present.""" current = self.settings.get(setting_key, "") - candidates = folder_paths.get(key, []) - if not isinstance(candidates, list) or not candidates: + primary_candidates = self._get_valid_root_candidates(key) + if not primary_candidates: return False - # Filter valid path strings - valid_paths = [p for p in candidates if isinstance(p, str) and p.strip()] - if not valid_paths: + allowed_roots = self._get_allowed_roots(key) + if current and current in allowed_roots: return False - if current in valid_paths: - return False - - self.settings[setting_key] = valid_paths[0] + self.settings[setting_key] = primary_candidates[0] if current: logger.info( - "Repaired stale %s from '%s' to '%s'", + "Repaired stale %s from '%s' to '%s' because it is not present in primary or extra roots", setting_key, current, - valid_paths[0], + primary_candidates[0], ) else: - logger.info("Auto-set %s to '%s'", setting_key, valid_paths[0]) + logger.info("Auto-set %s to '%s'", setting_key, primary_candidates[0]) return True # Process all model types @@ -813,6 +808,33 @@ class SettingsManager: else: self._save_settings() + def _get_valid_root_candidates(self, key: str) -> List[str]: + """Return stable root candidates, preferring primary roots over extra roots.""" + + candidates: List[str] = [] + seen: set[str] = set() + for mapping_key in ("folder_paths", "extra_folder_paths"): + raw_paths = self.settings.get(mapping_key, {}) + if not isinstance(raw_paths, Mapping): + continue + values = raw_paths.get(key, []) + if not isinstance(values, list): + continue + for value in values: + if not isinstance(value, str): + continue + normalized = value.strip() + if not normalized or normalized in seen: + continue + seen.add(normalized) + candidates.append(normalized) + return candidates + + def _get_allowed_roots(self, key: str) -> set[str]: + """Return all valid roots for a model type, including extra roots.""" + + return set(self._get_valid_root_candidates(key)) + def _check_environment_variables(self) -> None: """Check for environment variables and update settings if needed""" env_api_key = os.environ.get("CIVITAI_API_KEY") diff --git a/tests/config/test_config_save_paths.py b/tests/config/test_config_save_paths.py index 8fca10ed..0d2d13b8 100644 --- a/tests/config/test_config_save_paths.py +++ b/tests/config/test_config_save_paths.py @@ -46,6 +46,7 @@ def test_save_paths_renames_default_library(monkeypatch: pytest.MonkeyPatch, tmp self.delete_calls = [] self.upsert_calls = [] self._renamed = False + self.active_library = "default" def get_libraries(self): if self._renamed: @@ -62,6 +63,11 @@ def test_save_paths_renames_default_library(monkeypatch: pytest.MonkeyPatch, tmp def rename_library(self, old_name: str, new_name: str): self.rename_calls.append((old_name, new_name)) self._renamed = True + if self.active_library == old_name: + self.active_library = new_name + + def get_active_library_name(self): + return self.active_library def delete_library(self, name: str): # pragma: no cover - defensive guard self.delete_calls.append(name) @@ -104,6 +110,7 @@ def test_save_paths_logs_warning_when_upsert_fails( class RaisingSettingsService: def __init__(self): self.upsert_attempts = [] + self.active_library = "comfyui" def get_libraries(self): return { @@ -116,6 +123,9 @@ def test_save_paths_logs_warning_when_upsert_fails( 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.upsert_attempts.append((name, payload)) raise RuntimeError("boom") @@ -135,6 +145,8 @@ def test_save_paths_repairs_empty_default_roots(monkeypatch: pytest.MonkeyPatch, folder_paths = _setup_config_environment(monkeypatch, tmp_path) class FakeSettingsService: + active_library = "comfyui" + def get_libraries(self): return { "comfyui": { @@ -148,6 +160,9 @@ def test_save_paths_repairs_empty_default_roots(monkeypatch: pytest.MonkeyPatch, 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 @@ -167,6 +182,8 @@ def test_save_paths_repairs_stale_default_roots(monkeypatch: pytest.MonkeyPatch, folder_paths = _setup_config_environment(monkeypatch, tmp_path) class FakeSettingsService: + active_library = "comfyui" + def get_libraries(self): return { "comfyui": { @@ -180,6 +197,9 @@ def test_save_paths_repairs_stale_default_roots(monkeypatch: pytest.MonkeyPatch, 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 @@ -199,6 +219,8 @@ def test_save_paths_keeps_valid_default_roots(monkeypatch: pytest.MonkeyPatch, t folder_paths = _setup_config_environment(monkeypatch, tmp_path) class FakeSettingsService: + active_library = "comfyui" + def get_libraries(self): return { "comfyui": { @@ -212,6 +234,9 @@ def test_save_paths_keeps_valid_default_roots(monkeypatch: pytest.MonkeyPatch, t 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 @@ -258,6 +283,7 @@ def test_save_paths_removes_template_default_library(monkeypatch, tmp_path): self.rename_calls = [] self.delete_calls = [] self.upsert_calls = [] + self.active_library = "default" def get_libraries(self): return self.libraries @@ -265,6 +291,8 @@ def test_save_paths_removes_template_default_library(monkeypatch, tmp_path): def rename_library(self, old_name: str, new_name: str): self.rename_calls.append((old_name, new_name)) self.libraries[new_name] = self.libraries.pop(old_name) + if self.active_library == old_name: + self.active_library = new_name def delete_library(self, name: str): self.delete_calls.append(name) @@ -273,6 +301,11 @@ def test_save_paths_removes_template_default_library(monkeypatch, tmp_path): def upsert_library(self, name: str, **payload): self.upsert_calls.append((name, payload)) self.libraries[name] = {**payload} + if payload.get("activate"): + self.active_library = name + + def get_active_library_name(self): + return self.active_library fake_settings = FakeSettingsService() monkeypatch.setattr(settings_manager_module, "settings", fake_settings) @@ -313,6 +346,156 @@ def test_save_paths_removes_template_default_library(monkeypatch, tmp_path): assert payload["activate"] is True +def test_save_paths_keeps_default_roots_in_extra_paths(monkeypatch: pytest.MonkeyPatch, tmp_path): + folder_paths = _setup_config_environment(monkeypatch, tmp_path) + extra_lora_dir = tmp_path / "extra_loras" + extra_checkpoint_dir = tmp_path / "extra_checkpoints" + extra_embedding_dir = tmp_path / "extra_embeddings" + + for directory in (extra_lora_dir, extra_checkpoint_dir, extra_embedding_dir): + directory.mkdir() + + class FakeSettingsService: + active_library = "comfyui" + + def get_libraries(self): + return { + "comfyui": { + "folder_paths": {key: list(value) for key, value in folder_paths.items()}, + "extra_folder_paths": { + "loras": [str(extra_lora_dir)], + "checkpoints": [str(extra_checkpoint_dir)], + "embeddings": [str(extra_embedding_dir)], + }, + "default_lora_root": str(extra_lora_dir), + "default_checkpoint_root": str(extra_checkpoint_dir), + "default_embedding_root": str(extra_embedding_dir), + } + } + + 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_module.Config() + + assert fake_settings.name == "comfyui" + assert fake_settings.payload["extra_folder_paths"]["loras"] == [str(extra_lora_dir).replace("\\", "/")] + assert fake_settings.payload["extra_folder_paths"]["checkpoints"] == [ + str(extra_checkpoint_dir).replace("\\", "/") + ] + assert fake_settings.payload["extra_folder_paths"]["embeddings"] == [ + str(extra_embedding_dir).replace("\\", "/") + ] + assert fake_settings.payload["default_lora_root"] == str(extra_lora_dir).replace("\\", "/") + assert fake_settings.payload["default_checkpoint_root"] == str(extra_checkpoint_dir).replace("\\", "/") + assert fake_settings.payload["default_embedding_root"] == str(extra_embedding_dir).replace("\\", "/") + assert fake_settings.payload["activate"] is True + + +def test_save_paths_repairs_empty_default_roots_to_extra_paths_when_primary_missing( + monkeypatch: pytest.MonkeyPatch, tmp_path +): + _setup_config_environment(monkeypatch, tmp_path) + extra_lora_dir = tmp_path / "extra_loras" + extra_lora_dir.mkdir() + + monkeypatch.setattr( + config_module.folder_paths, + "get_folder_paths", + lambda kind: [] if kind == "loras" else [], + ) + + class FakeSettingsService: + active_library = "comfyui" + + def get_libraries(self): + return { + "comfyui": { + "folder_paths": { + "loras": [], + "checkpoints": [], + "unet": [], + "embeddings": [], + }, + "extra_folder_paths": { + "loras": [str(extra_lora_dir)], + }, + "default_lora_root": "", + } + } + + 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_module.Config() + + assert fake_settings.name == "comfyui" + assert fake_settings.payload["default_lora_root"] == str(extra_lora_dir).replace("\\", "/") + + +def test_save_paths_does_not_activate_comfyui_library_when_another_library_is_active( + monkeypatch: pytest.MonkeyPatch, tmp_path +): + folder_paths = _setup_config_environment(monkeypatch, tmp_path) + + class FakeSettingsService: + def __init__(self): + self.active_library = "studio" + self.upsert_calls = [] + + def get_libraries(self): + return { + "studio": { + "folder_paths": {"loras": ["/studio/loras"]}, + }, + "comfyui": { + "folder_paths": {key: list(value) for key, value in folder_paths.items()}, + "default_lora_root": folder_paths["loras"][0], + "default_checkpoint_root": folder_paths["checkpoints"][0], + "default_embedding_root": folder_paths["embeddings"][0], + }, + } + + 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.upsert_calls.append((name, payload)) + + fake_settings = FakeSettingsService() + monkeypatch.setattr(settings_manager_module, "settings", fake_settings) + + config_module.Config() + + assert len(fake_settings.upsert_calls) == 1 + name, payload = fake_settings.upsert_calls[0] + assert name == "comfyui" + assert payload["activate"] is False + + def test_apply_library_settings_merges_extra_paths(monkeypatch, tmp_path): """Test that apply_library_settings correctly merges folder_paths with extra_folder_paths.""" loras_dir = tmp_path / "loras" diff --git a/tests/services/test_settings_manager.py b/tests/services/test_settings_manager.py index 11c40089..7b14db91 100644 --- a/tests/services/test_settings_manager.py +++ b/tests/services/test_settings_manager.py @@ -332,6 +332,43 @@ def test_auto_set_default_roots_keeps_valid_values(manager): assert manager.get("default_embedding_root") == "/embeddings" +def test_auto_set_default_roots_keeps_valid_extra_values(manager): + manager.settings["default_lora_root"] = "/extra-loras" + manager.settings["default_checkpoint_root"] = "/extra-checkpoints" + manager.settings["default_embedding_root"] = "/extra-embeddings" + manager.settings["default_unet_root"] = "/extra-unet" + + manager.settings["folder_paths"] = { + "loras": ["/loras"], + "checkpoints": ["/checkpoints"], + "unet": ["/unet"], + "embeddings": ["/embeddings"], + } + manager.settings["extra_folder_paths"] = { + "loras": ["/extra-loras"], + "checkpoints": ["/extra-checkpoints"], + "unet": ["/extra-unet"], + "embeddings": ["/extra-embeddings"], + } + + manager._auto_set_default_roots() + + assert manager.get("default_lora_root") == "/extra-loras" + assert manager.get("default_checkpoint_root") == "/extra-checkpoints" + assert manager.get("default_unet_root") == "/extra-unet" + assert manager.get("default_embedding_root") == "/extra-embeddings" + + +def test_auto_set_default_roots_falls_back_to_extra_when_primary_missing(manager): + manager.settings["default_lora_root"] = "" + manager.settings["folder_paths"] = {"loras": []} + manager.settings["extra_folder_paths"] = {"loras": ["/extra-loras"]} + + manager._auto_set_default_roots() + + assert manager.get("default_lora_root") == "/extra-loras" + + def test_delete_setting(manager): manager.set("example", 1) manager.delete("example")