Refactor config resolution logic with layered fallbacks and robust override support

DEV_README.md

@@ -6,23 +6,37 @@ This guide provides an overview of how to develop in this repository.
 
 1. Clone the repo, create and activate a conda env. Minimum Python version is 3.9.
 
-2. Install the package to local
+2. Install the package to your conda env.
 
 `pip install -e .`
 
 3. Set ENVIRONMENT variable to DEV.
 
 `export ENVIRONMENT=dev`
 
-4. Test script running
+4. Check if the "comfy" package can run.
 
 `comfy --help`
 
-5. Use pre commit hook
+5. Install the pre-commit hook to ensure that your code won't need reformatting later.
 
 `pre-commit install`
 
-## Debug
+6. To save time during code review, it's recommended that you also manually run
+   the unit tests before submitting a pull request (see below).
+
+## Running the unit tests
+
+1. Install pytest into your conda env. You should preferably be using Python 3.9
+   in your conda env, since it's the version we are targeting for compatibility.
+
+`pip install pytest pytest-cov`
+
+2. Verify that all unit tests run successfully.
+
+`pytest --cov=comfy_cli --cov-report=xml .`
+
+## Debugging
 
 You can add following config to your VSCode `launch.json` to launch debugger.
 
@@ -37,14 +51,14 @@ You can add following config to your VSCode `launch.json` to launch debugger.
 }
 ```
 
-## Make changes to the code base
+## Making changes to the code base
 
 There is a potential need for you to reinstall the package. You can do this by
 either run `pip install -e .` again (which will reinstall), or manually
 uninstall `pip uninstall comfy-cli` and reinstall, or even cleaning your conda
 env and reinstalling the package (`pip install -e .`)
 
-## Add New Command
+## Adding a new command
 
 - Register it under `comfy_cli/cmdline.py`
 
@@ -77,18 +91,18 @@ def remove(name: str):
 
 ```
 
-## Guide
+## Important notes
 
 - Use `typer` for all command args management
 - Use `rich` for all console output
   - For progress reporting, use either [`rich.progress`](https://rich.readthedocs.io/en/stable/progress.html)
 
 ## Develop comfy-cli and ComfyUI-Manager (cm-cli) together
-### Make changes to both
+### Making changes to both
 1. Fork your own branches of `comfy-cli` and `ComfyUI-Manager`, make changes
 2. Be sure to commit any changes to `ComfyUI-Manager` to a new branch, and push to remote
 
-### Try out changes to both
+### Trying changes to both
 1. clone the changed branch of `comfy-cli`, then live install `comfy-cli`:
   - `pip install -e comfy-cli`
 2. Go to a test dir and run:
@@ -97,7 +111,7 @@ def remove(name: str):
   - `cd ComfyUI/custom_nodes/ComfyUI-Manager/ && git checkout <changed-branch> && cd -`
 4. Further changes can be pulled into these copies of the `comfy-cli` and `ComfyUI-Manager` repos
 
-### Debug both simultaneously
+### Debugging both simultaneously
 1. Follow instructions above to get working install with changes
 2. Add breakpoints directly to code: `import ipdb; ipdb.set_trace()`
 3. Execute relevant `comfy-cli` command

README.md

@@ -173,9 +173,11 @@ the bisect tool can help you pinpoint the custom node that causes the issue.
 
 - Model downloading
 
-  `comfy model download --url <URL> ?[--relative-path <PATH>] ?[--set-civitai-api-token <TOKEN>]`
+  `comfy model download --url <URL> ?[--relative-path <PATH>] ?[--set-civitai-api-token <TOKEN>] ?[--set-hf-api-token <TOKEN>]`
 
-  - URL: CivitAI, huggingface file url, ...
+  - URL: CivitAI page, Hugging Face file URL, etc...
+  - You can also specify your API tokens via the `CIVITAI_API_TOKEN` and `HF_API_TOKEN` environment variables. The order of priority is `--set-X-token` (always highest priority), then the environment variables if they exist, and lastly your config's stored tokens from previous `--set-X-token` usage (which remembers your most recently set token values).
+  - Tokens provided via the environment variables are never stored persistently in your config file. They are intended as a way to easily and safely provide transient secrets.
 
 - Model remove
 

comfy_cli/command/models/models.py

@@ -106,13 +106,13 @@ def check_civitai_url(url: str) -> Tuple[bool, bool, int, int]:
                 model_id = subpath.split("/")[1]
                 return True, False, int(model_id), None
     except (ValueError, IndexError):
-        print("Error parsing Civitai model URL")
+        print("Error parsing CivitAI model URL")
 
     return False, False, None, None
 
 
 def request_civitai_model_version_api(version_id: int, headers: Optional[dict] = None):
-    # Make a request to the Civitai API to get the model information
+    # Make a request to the CivitAI API to get the model information
     response = requests.get(
         f"https://civitai.com/api/v1/model-versions/{version_id}",
         headers=headers,
@@ -131,7 +131,7 @@ def request_civitai_model_version_api(version_id: int, headers: Optional[dict] =
 
 
 def request_civitai_model_api(model_id: int, version_id: int = None, headers: Optional[dict] = None):
-    # Make a request to the Civitai API to get the model information
+    # Make a request to the CivitAI API to get the model information
     response = requests.get(f"https://civitai.com/api/v1/models/{model_id}", headers=headers, timeout=10)
     response.raise_for_status()  # Raise an error for bad status codes
 
@@ -163,7 +163,7 @@ def download(
     _ctx: typer.Context,
     url: Annotated[
         str,
-        typer.Option(help="The URL from which to download the model", show_default=False),
+        typer.Option(help="The URL from which to download the model.", show_default=False),
     ],
     relative_path: Annotated[
         Optional[str],
@@ -183,15 +183,15 @@ def download(
         Optional[str],
         typer.Option(
             "--set-civitai-api-token",
-            help="Set the CivitAI API token to use for model listing.",
+            help="Set the CivitAI API token to use for model downloading.",
             show_default=False,
         ),
     ] = None,
     set_hf_api_token: Annotated[
         Optional[str],
         typer.Option(
             "--set-hf-api-token",
-            help="Set the HuggingFace API token to use for model listing.",
+            help="Set the Hugging Face API token to use for model downloading.",
             show_default=False,
         ),
     ] = None,
@@ -201,27 +201,23 @@ def download(
 
     local_filename = None
     headers = None
-    civitai_api_token = None
 
-    if set_civitai_api_token is not None:
-        config_manager.set(constants.CIVITAI_API_TOKEN_KEY, set_civitai_api_token)
-        civitai_api_token = set_civitai_api_token
+    civitai_api_token = config_manager.get_or_override(
+        constants.CIVITAI_API_TOKEN_ENV_KEY, constants.CIVITAI_API_TOKEN_KEY, set_civitai_api_token
+    )
+    hf_api_token = config_manager.get_or_override(
+        constants.HF_API_TOKEN_ENV_KEY, constants.HF_API_TOKEN_KEY, set_hf_api_token
+    )
 
-    if set_hf_api_token is not None:
-        config_manager.set(constants.HF_API_TOKEN_KEY, set_hf_api_token)
-        hf_api_token = set_hf_api_token
-    else:
-        civitai_api_token = config_manager.get(constants.CIVITAI_API_TOKEN_KEY)
-        hf_api_token = config_manager.get(constants.HF_API_TOKEN_KEY)
+    is_civitai_model_url, is_civitai_api_url, model_id, version_id = check_civitai_url(url)
+    is_huggingface_url, repo_id, hf_filename, hf_folder_name, hf_branch_name = check_huggingface_url(url)
 
-    if civitai_api_token is not None:
+    if is_civitai_model_url or is_civitai_api_url:
         headers = {
             "Content-Type": "application/json",
-            "Authorization": f"Bearer {civitai_api_token}",
         }
-
-    is_civitai_model_url, is_civitai_api_url, model_id, version_id = check_civitai_url(url)
-    is_huggingface_url, repo_id, hf_filename, hf_folder_name, hf_branch_name = check_huggingface_url(url)
+        if civitai_api_token is not None:
+            headers["Authorization"] = f"Bearer {civitai_api_token}"
 
     if is_civitai_model_url:
         local_filename, url, model_type, basemodel = request_civitai_model_api(model_id, version_id, headers)
@@ -280,7 +276,7 @@ def download(
     if is_huggingface_url and check_unauthorized(url, headers):
         if hf_api_token is None:
             print(
-                "Unauthorized access to Hugging Face model. Please set the HuggingFace API token using --set-hf-api-token"
+                f"Unauthorized access to Hugging Face model. Please set the Hugging Face API token using `comfy model download --set-hf-api-token` or via the `{constants.HF_API_TOKEN_ENV_KEY}` environment variable"
             )
             return
         else:

comfy_cli/config_manager.py

@@ -43,6 +43,27 @@ def get(self, key):
         """
         return self.config["DEFAULT"].get(key, None)  # Returns None if the key does not exist
 
+    def get_or_override(self, env_key: str, config_key: str, set_value: Optional[str] = None) -> Optional[str]:
+        """
+        Resolves and conditionally stores a config value.
+
+        The selected value and action is determined by the following priority:
+
+        1. Use CLI-provided `--set-*` value (if not None), and save it to config via `set()`.
+        2. Use process environment variable if exists (empty strings are allowed).
+        3. Otherwise, use the current config value via `get()`.
+
+        Returns None if the selected value is an empty string.
+        """
+
+        if set_value is not None:
+            self.set(config_key, set_value)
+            return set_value or None
+        elif env_key in os.environ:
+            return os.environ[env_key] or None
+        else:
+            return self.get(config_key) or None
+
     def load(self):
         config_file_path = self.get_config_file_path()
         if os.path.exists(config_file_path):

comfy_cli/constants.py

@@ -42,7 +42,9 @@ class PROC(str, Enum):
 CONFIG_KEY_BACKGROUND = "background"
 
 CIVITAI_API_TOKEN_KEY = "civitai_api_token"
+CIVITAI_API_TOKEN_ENV_KEY = "CIVITAI_API_TOKEN"
 HF_API_TOKEN_KEY = "hf_api_token"
+HF_API_TOKEN_ENV_KEY = "HF_API_TOKEN"
 
 DEFAULT_TRACKING_VALUE = True
 

comfy_cli/file_utils.py

@@ -7,7 +7,7 @@
 import httpx
 import requests
 
-from comfy_cli import ui
+from comfy_cli import constants, ui
 
 
 class DownloadException(Exception):
@@ -36,12 +36,12 @@ def parse_json(input_data):
         msg_json = parse_json(message)
         if msg_json is not None:
             if "message" in msg_json:
-                return f"Unauthorized download ({status_code}).\n{msg_json['message']}\nor you can set civitai api token using `comfy model download --set-civitai-api-token <token>`"
-        return f"Unauthorized download ({status_code}), you might need to manually log into browser to download one"
+                return f"Unauthorized download ({status_code}).\n{msg_json['message']}\nor you can set a CivitAI API token using `comfy model download --set-civitai-api-token` or via the `{constants.CIVITAI_API_TOKEN_ENV_KEY}` environment variable"
+        return f"Unauthorized download ({status_code}), you might need to manually log into a browser to download this"
     elif status_code == 403:
-        return f"Forbidden url ({status_code}), you might need to manually log into browser to download one"
+        return f"Forbidden url ({status_code}), you might need to manually log into a browser to download this"
     elif status_code == 404:
-        return "Sorry, your file is in another castle (404)"
+        return "File not found on server (404)"
     return f"Unknown error occurred (status code: {status_code})"
 
 

tests/test_file_utils.py

@@ -25,7 +25,7 @@ def test_guess_status_code_reason_401_with_json():
 def test_guess_status_code_reason_401_without_json():
     result = guess_status_code_reason(401, "not json")
     assert "Unauthorized download (401)" in result
-    assert "manually log into browser" in result
+    assert "manually log into a browser" in result
 
 
 def test_guess_status_code_reason_403():
@@ -35,7 +35,7 @@ def test_guess_status_code_reason_403():
 
 def test_guess_status_code_reason_404():
     result = guess_status_code_reason(404, "")
-    assert "another castle (404)" in result
+    assert "not found on server (404)" in result
 
 
 def test_guess_status_code_reason_unknown():