From e488f8bbc4978dea4500e286a038fcab38ba3895 Mon Sep 17 00:00:00 2001 From: cest-la-v <22951622+cest-la-v@users.noreply.github.com> Date: Sun, 26 Apr 2026 20:34:30 +0800 Subject: [PATCH] fix: address review feedback on extra_paths.yaml loader - extra_model_paths.yaml is now ignored (not merged) when extra_paths.yaml exists; warning message clarified to instruct deletion/migration - System directory keys (output/input/temp/user) now gated behind allow_system_dirs=True; only extra_paths.yaml sets this flag, so legacy extra_model_paths.yaml files cannot accidentally call set_*_directory() if a block happens to be named 'output' etc. - Add test_system_dir_keys_not_applied_for_legacy to cover the guard --- main.py | 10 +++++----- tests-unit/utils/extra_config_test.py | 24 +++++++++++++++++++++++- utils/extra_config.py | 4 ++-- 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/main.py b/main.py index 359605f41..54d0bfa8e 100644 --- a/main.py +++ b/main.py @@ -100,14 +100,14 @@ def apply_custom_paths(): extra_model_paths_config_path = os.path.join(install_dir, "extra_model_paths.yaml") if os.path.isfile(extra_paths_config_path): - utils.extra_config.load_extra_path_config(extra_paths_config_path) + utils.extra_config.load_extra_path_config(extra_paths_config_path, allow_system_dirs=True) if os.path.isfile(extra_model_paths_config_path): logging.warning( - "Both extra_paths.yaml and extra_model_paths.yaml found. " - "extra_model_paths.yaml is deprecated; please migrate to extra_paths.yaml." + "Both extra_paths.yaml and extra_model_paths.yaml found; " + "ignoring the deprecated extra_model_paths.yaml. " + "Please remove or migrate its entries to extra_paths.yaml." ) - - if os.path.isfile(extra_model_paths_config_path): + elif os.path.isfile(extra_model_paths_config_path): utils.extra_config.load_extra_path_config(extra_model_paths_config_path) if args.extra_model_paths_config: diff --git a/tests-unit/utils/extra_config_test.py b/tests-unit/utils/extra_config_test.py index baa9e1d22..1112320d9 100644 --- a/tests-unit/utils/extra_config_test.py +++ b/tests-unit/utils/extra_config_test.py @@ -429,7 +429,7 @@ def test_system_dir_keys(mock_yaml_load, save_restore_system_dirs, tmp_path): with open(yaml_path, "w") as f: f.write("") - load_extra_path_config(yaml_path) + load_extra_path_config(yaml_path, allow_system_dirs=True) assert folder_paths.get_output_directory() == os.path.normpath(str(tmp_path / "my_output")) assert folder_paths.get_input_directory() == os.path.normpath(str(tmp_path / "my_input")) @@ -437,6 +437,28 @@ def test_system_dir_keys(mock_yaml_load, save_restore_system_dirs, tmp_path): assert folder_paths.get_user_directory() == os.path.normpath(str(tmp_path / "my_user")) +@patch("yaml.safe_load") +def test_system_dir_keys_not_applied_for_legacy(mock_yaml_load, save_restore_system_dirs, tmp_path): + """System directory keys are ignored when allow_system_dirs=False (legacy extra_model_paths.yaml).""" + original_output = folder_paths.get_output_directory() + config_data = { + "comfyui": { + "base_path": str(tmp_path), + "output": "my_output/", + } + } + mock_yaml_load.return_value = config_data + + yaml_path = str(tmp_path / "extra_model_paths.yaml") + with open(yaml_path, "w") as f: + f.write("") + + load_extra_path_config(yaml_path) # allow_system_dirs defaults to False + + # output should be unchanged — treated as a model category, not a system dir + assert folder_paths.get_output_directory() == original_output + + @patch("yaml.safe_load") def test_nested_models_block(mock_yaml_load, clear_folder_paths, tmp_path): """Nested models: block registers model paths relative to models/base_path.""" diff --git a/utils/extra_config.py b/utils/extra_config.py index 1f957ad2b..8ba1bdc38 100644 --- a/utils/extra_config.py +++ b/utils/extra_config.py @@ -49,7 +49,7 @@ def _implicit_scan(base: str, exclude: set[str], is_default: bool) -> None: folder_paths.add_model_folder_path(category, path, is_default) -def load_extra_path_config(yaml_path: str) -> None: +def load_extra_path_config(yaml_path: str, allow_system_dirs: bool = False) -> None: with open(yaml_path, 'r', encoding='utf-8') as stream: config = yaml.safe_load(stream) yaml_dir = os.path.dirname(os.path.abspath(yaml_path)) @@ -68,7 +68,7 @@ def load_extra_path_config(yaml_path: str) -> None: flat_model_keys: set[str] = set() for key, value in conf.items(): - if key in _SYSTEM_DIR_KEYS: + if allow_system_dirs and key in _SYSTEM_DIR_KEYS: # System directory override → set_*_directory() path = _resolve_base(str(value).strip(), block_base, yaml_dir) logging.info("Setting %s directory to %s", key, path)