From ae457da84bfaf074d68b73881c565e3dd2b20b98 Mon Sep 17 00:00:00 2001 From: Jedrzej Kosinski Date: Mon, 4 May 2026 19:50:26 -0700 Subject: [PATCH] feat: add generic --feature-flag CLI arg and --list-feature-flags registry (#13685) --- comfy/cli_args.py | 2 + comfy_api/feature_flags.py | 92 +++++++++++++++++++++++++++++++- main.py | 10 +++- tests-unit/feature_flags_test.py | 85 +++++++++++++++++++++++++++++ 4 files changed, 186 insertions(+), 3 deletions(-) diff --git a/comfy/cli_args.py b/comfy/cli_args.py index d2fde8b67..9dadb0093 100644 --- a/comfy/cli_args.py +++ b/comfy/cli_args.py @@ -238,6 +238,8 @@ database_default_path = os.path.abspath( ) parser.add_argument("--database-url", type=str, default=f"sqlite:///{database_default_path}", help="Specify the database URL, e.g. for an in-memory database you can use 'sqlite:///:memory:'.") parser.add_argument("--enable-assets", action="store_true", help="Enable the assets system (API routes, database synchronization, and background scanning).") +parser.add_argument("--feature-flag", type=str, action='append', default=[], metavar="KEY[=VALUE]", help="Set a server feature flag. Use KEY=VALUE to set an explicit value, or bare KEY to set it to true. Can be specified multiple times. Boolean values (true/false) and numbers are auto-converted. Examples: --feature-flag show_signin_button=true or --feature-flag show_signin_button") +parser.add_argument("--list-feature-flags", action="store_true", help="Print the registry of known CLI-settable feature flags as JSON and exit.") if comfy.options.args_parsing: args = parser.parse_args() diff --git a/comfy_api/feature_flags.py b/comfy_api/feature_flags.py index 9f6918315..adb5a3144 100644 --- a/comfy_api/feature_flags.py +++ b/comfy_api/feature_flags.py @@ -5,12 +5,95 @@ This module handles capability negotiation between frontend and backend, allowing graceful protocol evolution while maintaining backward compatibility. """ -from typing import Any +import logging +from typing import Any, TypedDict from comfy.cli_args import args + +class FeatureFlagInfo(TypedDict): + type: str + default: Any + description: str + + +# Registry of known CLI-settable feature flags. +# Launchers can query this via --list-feature-flags to discover valid flags. +CLI_FEATURE_FLAG_REGISTRY: dict[str, FeatureFlagInfo] = { + "show_signin_button": { + "type": "bool", + "default": False, + "description": "Show the sign-in button in the frontend even when not signed in", + }, +} + + +def _coerce_bool(v: str) -> bool: + """Strict bool coercion: only 'true'/'false' (case-insensitive). + + Anything else raises ValueError so the caller can warn and drop the flag, + rather than silently treating typos like 'ture' or 'yes' as False. + """ + lower = v.lower() + if lower == "true": + return True + if lower == "false": + return False + raise ValueError(f"expected 'true' or 'false', got {v!r}") + + +_COERCE_FNS: dict[str, Any] = { + "bool": _coerce_bool, + "int": lambda v: int(v), + "float": lambda v: float(v), +} + + +def _coerce_flag_value(key: str, raw_value: str) -> Any: + """Coerce a raw string value using the registry type, or keep as string. + + Returns the raw string if the key is unregistered or the type is unknown. + Raises ValueError/TypeError if the key is registered with a known type but + the value cannot be coerced; callers are expected to warn and drop the flag. + """ + info = CLI_FEATURE_FLAG_REGISTRY.get(key) + if info is None: + return raw_value + coerce = _COERCE_FNS.get(info["type"]) + if coerce is None: + return raw_value + return coerce(raw_value) + + +def _parse_cli_feature_flags() -> dict[str, Any]: + """Parse --feature-flag key=value pairs from CLI args into a dict. + + Items without '=' default to the value 'true' (bare flag form). + Flags whose value cannot be coerced to the registered type are dropped + with a warning, so a typo like '--feature-flag some_bool=ture' does not + silently take effect as the wrong value. + """ + result: dict[str, Any] = {} + for item in getattr(args, "feature_flag", []): + key, sep, raw_value = item.partition("=") + key = key.strip() + if not key: + continue + if not sep: + raw_value = "true" + try: + result[key] = _coerce_flag_value(key, raw_value.strip()) + except (ValueError, TypeError) as e: + info = CLI_FEATURE_FLAG_REGISTRY.get(key, {}) + logging.warning( + "Could not coerce --feature-flag %s=%r to %s (%s); dropping flag.", + key, raw_value.strip(), info.get("type", "?"), e, + ) + return result + + # Default server capabilities -SERVER_FEATURE_FLAGS: dict[str, Any] = { +_CORE_FEATURE_FLAGS: dict[str, Any] = { "supports_preview_metadata": True, "max_upload_size": args.max_upload_size * 1024 * 1024, # Convert MB to bytes "extension": {"manager": {"supports_v4": True}}, @@ -18,6 +101,11 @@ SERVER_FEATURE_FLAGS: dict[str, Any] = { "assets": args.enable_assets, } +# CLI-provided flags cannot overwrite core flags +_cli_flags = {k: v for k, v in _parse_cli_feature_flags().items() if k not in _CORE_FEATURE_FLAGS} + +SERVER_FEATURE_FLAGS: dict[str, Any] = {**_CORE_FEATURE_FLAGS, **_cli_flags} + def get_connection_feature( sockets_metadata: dict[str, dict[str, Any]], diff --git a/main.py b/main.py index dbaf2745c..a6fdaf43c 100644 --- a/main.py +++ b/main.py @@ -1,13 +1,21 @@ import comfy.options comfy.options.enable_args_parsing() +from comfy.cli_args import args + +if args.list_feature_flags: + import json + from comfy_api.feature_flags import CLI_FEATURE_FLAG_REGISTRY + print(json.dumps(CLI_FEATURE_FLAG_REGISTRY, indent=2)) # noqa: T201 + raise SystemExit(0) + import os import importlib.util import shutil import importlib.metadata import folder_paths import time -from comfy.cli_args import args, enables_dynamic_vram +from comfy.cli_args import enables_dynamic_vram from app.logger import setup_logger setup_logger(log_level=args.verbose, use_stdout=args.log_stdout) diff --git a/tests-unit/feature_flags_test.py b/tests-unit/feature_flags_test.py index f2702cfc8..8ec52a124 100644 --- a/tests-unit/feature_flags_test.py +++ b/tests-unit/feature_flags_test.py @@ -1,10 +1,15 @@ """Tests for feature flags functionality.""" +import pytest + from comfy_api.feature_flags import ( get_connection_feature, supports_feature, get_server_features, + CLI_FEATURE_FLAG_REGISTRY, SERVER_FEATURE_FLAGS, + _coerce_flag_value, + _parse_cli_feature_flags, ) @@ -96,3 +101,83 @@ class TestFeatureFlags: result = get_connection_feature(sockets_metadata, "sid1", "any_feature") assert result is False assert supports_feature(sockets_metadata, "sid1", "any_feature") is False + + +class TestCoerceFlagValue: + """Test suite for _coerce_flag_value.""" + + def test_registered_bool_true(self): + assert _coerce_flag_value("show_signin_button", "true") is True + assert _coerce_flag_value("show_signin_button", "True") is True + + def test_registered_bool_false(self): + assert _coerce_flag_value("show_signin_button", "false") is False + assert _coerce_flag_value("show_signin_button", "FALSE") is False + + def test_unregistered_key_stays_string(self): + assert _coerce_flag_value("unknown_flag", "true") == "true" + assert _coerce_flag_value("unknown_flag", "42") == "42" + + def test_bool_typo_raises(self): + """Strict bool: typos like 'ture' or 'yes' must raise so the flag can be dropped.""" + with pytest.raises(ValueError): + _coerce_flag_value("show_signin_button", "ture") + with pytest.raises(ValueError): + _coerce_flag_value("show_signin_button", "yes") + with pytest.raises(ValueError): + _coerce_flag_value("show_signin_button", "1") + with pytest.raises(ValueError): + _coerce_flag_value("show_signin_button", "") + + def test_failed_int_coercion_raises(self, monkeypatch): + """Malformed values for typed flags must raise; caller decides what to do.""" + monkeypatch.setitem( + CLI_FEATURE_FLAG_REGISTRY, + "test_int_flag", + {"type": "int", "default": 0, "description": "test"}, + ) + with pytest.raises(ValueError): + _coerce_flag_value("test_int_flag", "not_a_number") + + +class TestParseCliFeatureFlags: + """Test suite for _parse_cli_feature_flags.""" + + def test_single_flag(self, monkeypatch): + monkeypatch.setattr("comfy_api.feature_flags.args", type("Args", (), {"feature_flag": ["show_signin_button=true"]})()) + result = _parse_cli_feature_flags() + assert result == {"show_signin_button": True} + + def test_missing_equals_defaults_to_true(self, monkeypatch): + """Bare flag without '=' is treated as the string 'true' (and coerced if registered).""" + monkeypatch.setattr("comfy_api.feature_flags.args", type("Args", (), {"feature_flag": ["show_signin_button", "valid=1"]})()) + result = _parse_cli_feature_flags() + assert result == {"show_signin_button": True, "valid": "1"} + + def test_empty_key_skipped(self, monkeypatch): + monkeypatch.setattr("comfy_api.feature_flags.args", type("Args", (), {"feature_flag": ["=value", "valid=1"]})()) + result = _parse_cli_feature_flags() + assert result == {"valid": "1"} + + def test_invalid_bool_value_dropped(self, monkeypatch, caplog): + """A typo'd bool value must be dropped entirely, not silently set to False + and not stored as a raw string. A warning must be logged.""" + monkeypatch.setattr( + "comfy_api.feature_flags.args", + type("Args", (), {"feature_flag": ["show_signin_button=ture", "valid=1"]})(), + ) + with caplog.at_level("WARNING"): + result = _parse_cli_feature_flags() + assert result == {"valid": "1"} + assert "show_signin_button" not in result + assert any("show_signin_button" in r.message and "drop" in r.message.lower() for r in caplog.records) + + +class TestCliFeatureFlagRegistry: + """Test suite for the CLI feature flag registry.""" + + def test_registry_entries_have_required_fields(self): + for key, info in CLI_FEATURE_FLAG_REGISTRY.items(): + assert "type" in info, f"{key} missing 'type'" + assert "default" in info, f"{key} missing 'default'" + assert "description" in info, f"{key} missing 'description'"