Compare commits

..

8 Commits

Author SHA1 Message Date
f42bede3c3 Merge branch 'master' into feature/generic-feature-flag-cli 2026-05-04 19:29:05 -07:00
7d4fb0929c --feature-flag: strict bool coercion + drop invalid values
Per @rattus128 review on PR #13685: silently coercing typo'd bool values
(e.g. `--feature-flag show_signin_button=ture`) to `False` was a confusing
UX. Make bool coercion strict and drop unparseable flags entirely.

- `_coerce_bool`: accept only `true`/`false` (case-insensitive); raise
  `ValueError` for anything else (`ture`, `yes`, `1`, ``).
- `_coerce_flag_value`: no longer swallows exceptions; raises on bad
  coercion so the caller decides what to do.
- `_parse_cli_feature_flags`: catches `ValueError`/`TypeError`, logs a
  warning ("dropping flag"), and omits the flag from the result. ComfyUI
  still starts; `SERVER_FEATURE_FLAGS` retains the registered default;
  other valid `--feature-flag` entries on the same command line are
  unaffected.

Tests:
- `test_bool_typo_raises`: `ture`/`yes`/`1`/`""` all raise ValueError.
- `test_failed_int_coercion_raises`: replaces the old "falls back to
  string" test now that coercion failures propagate.
- `test_invalid_bool_value_dropped`: parser drops the bad flag and logs
  a warning, while a valid sibling flag still parses.

19/19 unit tests pass.

Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019df5a8-36be-7107-a4af-c7e4f51687df
2026-05-04 18:19:16 -07:00
0b5da2af97 Merge branch 'master' into feature/generic-feature-flag-cli 2026-05-04 18:17:50 -07:00
ce2f848fa2 --feature-flag: document bare-key shorthand in metavar and help
Per CodeRabbit suggestion, the metavar 'KEY=VALUE' and help text omitted the supported '--feature-flag KEY' (no '=value', implicitly true) form. Update metavar to 'KEY[=VALUE]' and rewrite the help string to mention both forms with examples.

Amp-Thread-ID: https://ampcode.com/threads/T-019df26e-96f4-7518-94da-0e4263680e3c
Co-authored-by: Amp <amp@ampcode.com>
2026-05-04 07:46:44 -07:00
0992141135 Merge branch 'master' into feature/generic-feature-flag-cli 2026-05-04 07:37:45 -07:00
d187c3510e fix(feature-flags): bare flags default to true, robust coercion, drop wrapper
Address code review feedback:
- _coerce_flag_value: wrap coercion in try/except (ValueError, TypeError)
  and log a warning instead of crashing startup on malformed values.
- _parse_cli_feature_flags: bare --feature-flag KEY (no '=') now defaults
  to 'true' so registered bool flags work as toggles.
- Remove the get_cli_feature_flag_registry() wrapper; export and use
  CLI_FEATURE_FLAG_REGISTRY directly in main.py and tests.

Add tests for coercion-failure fallback and bare-flag default behavior.

Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019deba2-bfe2-7118-913c-562beee48972
2026-05-03 04:49:22 -07:00
393248c8fa Merge remote-tracking branch 'origin/master' into feature/generic-feature-flag-cli 2026-05-02 19:21:33 -07:00
45762f72a8 feat: add generic --feature-flag CLI arg and --list-feature-flags registry
Add --feature-flag KEY=VALUE CLI argument that allows setting arbitrary
server feature flags at startup. Values are auto-converted to appropriate
Python types (bool, int, float, string). CLI flags are merged into
SERVER_FEATURE_FLAGS but cannot overwrite core flags.

Add --list-feature-flags which prints the registry of known CLI-settable
feature flags as JSON and exits, enabling launchers to discover valid
flags for a specific ComfyUI version.

Part of Comfy-Org/ComfyUI-Desktop-2.0-Beta#415

Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019d9386-54d3-74d9-a661-97e0a8d37b6b
2026-04-15 22:46:18 -07:00
4 changed files with 0 additions and 147 deletions

1
.gitignore vendored
View File

@ -23,4 +23,3 @@ web_custom_versions/
.DS_Store
filtered-openapi.yaml
uv.lock
.comfy_environment

View File

@ -1,34 +0,0 @@
import functools
import logging
import os
logger = logging.getLogger(__name__)
_DEFAULT_DEPLOY_ENV = "local-git"
_ENV_FILENAME = ".comfy_environment"
# Resolve the ComfyUI install directory (the parent of this `comfy/` package).
# We deliberately avoid `folder_paths.base_path` here because that is overridden
# by the `--base-directory` CLI arg to a user-supplied path, whereas the
# `.comfy_environment` marker is written by launchers/installers next to the
# ComfyUI install itself.
_COMFY_INSTALL_DIR = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))
@functools.cache
def get_deploy_environment() -> str:
env_file = os.path.join(_COMFY_INSTALL_DIR, _ENV_FILENAME)
try:
with open(env_file, encoding="utf-8") as f:
# Cap the read so a malformed or maliciously crafted file (e.g.
# a single huge line with no newline) can't blow up memory.
first_line = f.readline(128).strip()
value = "".join(c for c in first_line if 32 <= ord(c) < 127)
if value:
return value
except FileNotFoundError:
pass
except Exception as e:
logger.error("Failed to read %s: %s", env_file, e)
return _DEFAULT_DEPLOY_ENV

View File

@ -19,8 +19,6 @@ from comfy import utils
from comfy_api.latest import IO
from server import PromptServer
from comfy.deploy_environment import get_deploy_environment
from . import request_logger
from ._helpers import (
default_base_url,
@ -626,7 +624,6 @@ async def _request_base(cfg: _RequestConfig, expect_binary: bool):
payload_headers = {"Accept": "*/*"} if expect_binary else {"Accept": "application/json"}
if not parsed_url.scheme and not parsed_url.netloc: # is URL relative?
payload_headers.update(get_auth_header(cfg.node_cls))
payload_headers["Comfy-Env"] = get_deploy_environment()
if cfg.endpoint.headers:
payload_headers.update(cfg.endpoint.headers)

View File

@ -1,109 +0,0 @@
"""Tests for comfy.deploy_environment."""
import os
import pytest
from comfy import deploy_environment
from comfy.deploy_environment import get_deploy_environment
@pytest.fixture(autouse=True)
def _reset_cache_and_install_dir(tmp_path, monkeypatch):
"""Reset the functools cache and point the ComfyUI install dir at a tmp dir for each test."""
get_deploy_environment.cache_clear()
monkeypatch.setattr(deploy_environment, "_COMFY_INSTALL_DIR", str(tmp_path))
yield
get_deploy_environment.cache_clear()
def _write_env_file(tmp_path, content: str) -> str:
"""Write the env file with exact content (no newline translation).
`newline=""` disables Python's text-mode newline translation so the bytes
on disk match the literal string passed in, regardless of host OS.
Newline-style tests (CRLF, lone CR) rely on this.
"""
path = os.path.join(str(tmp_path), ".comfy_environment")
with open(path, "w", encoding="utf-8", newline="") as f:
f.write(content)
return path
class TestGetDeployEnvironment:
def test_returns_local_git_when_file_missing(self):
assert get_deploy_environment() == "local-git"
def test_reads_value_from_file(self, tmp_path):
_write_env_file(tmp_path, "local-desktop2-standalone\n")
assert get_deploy_environment() == "local-desktop2-standalone"
def test_strips_trailing_whitespace_and_newline(self, tmp_path):
_write_env_file(tmp_path, " local-desktop2-standalone \n")
assert get_deploy_environment() == "local-desktop2-standalone"
def test_only_first_line_is_used(self, tmp_path):
_write_env_file(tmp_path, "first-line\nsecond-line\n")
assert get_deploy_environment() == "first-line"
def test_crlf_line_ending(self, tmp_path):
# Windows editors often save text files with CRLF line endings.
# The CR must not end up in the returned value.
_write_env_file(tmp_path, "local-desktop2-standalone\r\n")
assert get_deploy_environment() == "local-desktop2-standalone"
def test_crlf_multiline_only_first_line_used(self, tmp_path):
_write_env_file(tmp_path, "first-line\r\nsecond-line\r\n")
assert get_deploy_environment() == "first-line"
def test_crlf_with_surrounding_whitespace(self, tmp_path):
_write_env_file(tmp_path, " local-desktop2-standalone \r\n")
assert get_deploy_environment() == "local-desktop2-standalone"
def test_lone_cr_line_ending(self, tmp_path):
# Classic-Mac / some legacy editors use a bare CR.
# Universal-newlines decoding treats it as a line terminator too.
_write_env_file(tmp_path, "local-desktop2-standalone\r")
assert get_deploy_environment() == "local-desktop2-standalone"
def test_empty_file_falls_back_to_default(self, tmp_path):
_write_env_file(tmp_path, "")
assert get_deploy_environment() == "local-git"
def test_empty_after_whitespace_strip_falls_back_to_default(self, tmp_path):
_write_env_file(tmp_path, " \n")
assert get_deploy_environment() == "local-git"
def test_strips_control_chars_within_first_line(self, tmp_path):
# Embedded NUL/control chars in the value should be stripped
# (header-injection / smuggling protection).
_write_env_file(tmp_path, "abc\x00\x07xyz\n")
assert get_deploy_environment() == "abcxyz"
def test_strips_non_ascii_characters(self, tmp_path):
_write_env_file(tmp_path, "café-é\n")
assert get_deploy_environment() == "caf-"
def test_caps_read_at_128_bytes(self, tmp_path):
# A single huge line with no newline must not be fully read into memory.
huge = "x" * 10_000
_write_env_file(tmp_path, huge)
result = get_deploy_environment()
assert result == "x" * 128
def test_result_is_cached_across_calls(self, tmp_path):
path = _write_env_file(tmp_path, "first_value\n")
assert get_deploy_environment() == "first_value"
# Overwrite the file — cached value should still be returned.
with open(path, "w", encoding="utf-8") as f:
f.write("second_value\n")
assert get_deploy_environment() == "first_value"
def test_unreadable_file_falls_back_to_default(self, tmp_path, monkeypatch):
_write_env_file(tmp_path, "should_not_be_used\n")
def _boom(*args, **kwargs):
raise OSError("simulated read failure")
monkeypatch.setattr("builtins.open", _boom)
assert get_deploy_environment() == "local-git"