From 45762f72a8115f6b496c260a26d7dd2a74565b9c Mon Sep 17 00:00:00 2001 From: Jedrzej Kosinski Date: Wed, 15 Apr 2026 17:06:59 -0700 Subject: [PATCH 1/2] 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-Thread-ID: https://ampcode.com/threads/T-019d9386-54d3-74d9-a661-97e0a8d37b6b --- comfy/cli_args.py | 2 + comfy_api/feature_flags.py | 63 +++++++++++++++++++++++++++++++- main.py | 10 ++++- tests-unit/feature_flags_test.py | 43 ++++++++++++++++++++++ 4 files changed, 115 insertions(+), 3 deletions(-) diff --git a/comfy/cli_args.py b/comfy/cli_args.py index dbaadf723..36552a1e3 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 as a key=value pair. Can be specified multiple times. Boolean values (true/false) and numbers are auto-converted. Example: --feature-flag show_signin_button=true") +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..66d37668d 100644 --- a/comfy_api/feature_flags.py +++ b/comfy_api/feature_flags.py @@ -5,12 +5,66 @@ This module handles capability negotiation between frontend and backend, allowing graceful protocol evolution while maintaining backward compatibility. """ -from typing import Any +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 get_cli_feature_flag_registry() -> dict[str, FeatureFlagInfo]: + """Return the registry of known CLI-settable feature flags.""" + return {k: dict(v) for k, v in CLI_FEATURE_FLAG_REGISTRY.items()} + + +_COERCE_FNS: dict[str, Any] = { + "bool": lambda v: v.lower() == "true", + "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.""" + 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.""" + result: dict[str, Any] = {} + for item in getattr(args, "feature_flag", []): + if "=" not in item: + continue + key, _, raw_value = item.partition("=") + key = key.strip() + if key: + result[key] = _coerce_flag_value(key, raw_value.strip()) + 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 +72,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 12b04719d..4a1af63db 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 get_cli_feature_flag_registry + print(json.dumps(get_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 from app.assets.seeder import asset_seeder from app.assets.services import register_output_files diff --git a/tests-unit/feature_flags_test.py b/tests-unit/feature_flags_test.py index f2702cfc8..34f14818e 100644 --- a/tests-unit/feature_flags_test.py +++ b/tests-unit/feature_flags_test.py @@ -4,7 +4,10 @@ from comfy_api.feature_flags import ( get_connection_feature, supports_feature, get_server_features, + get_cli_feature_flag_registry, SERVER_FEATURE_FLAGS, + _coerce_flag_value, + _parse_cli_feature_flags, ) @@ -96,3 +99,43 @@ 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" + + +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_skipped(self, monkeypatch): + monkeypatch.setattr("comfy_api.feature_flags.args", type("Args", (), {"feature_flag": ["noequals", "valid=1"]})()) + result = _parse_cli_feature_flags() + assert result == {"valid": "1"} + + +class TestCliFeatureFlagRegistry: + """Test suite for the CLI feature flag registry.""" + + def test_registry_entries_have_required_fields(self): + for key, info in get_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'" From d187c3510e1cd2874cdaef386f3d631e56d06475 Mon Sep 17 00:00:00 2001 From: Jedrzej Kosinski Date: Sun, 3 May 2026 04:49:22 -0700 Subject: [PATCH 2/2] 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-Thread-ID: https://ampcode.com/threads/T-019deba2-bfe2-7118-913c-562beee48972 --- comfy_api/feature_flags.py | 37 +++++++++++++++++++++----------- main.py | 4 ++-- tests-unit/feature_flags_test.py | 23 ++++++++++++++++---- 3 files changed, 45 insertions(+), 19 deletions(-) diff --git a/comfy_api/feature_flags.py b/comfy_api/feature_flags.py index 66d37668d..c370cf01d 100644 --- a/comfy_api/feature_flags.py +++ b/comfy_api/feature_flags.py @@ -5,6 +5,7 @@ This module handles capability negotiation between frontend and backend, allowing graceful protocol evolution while maintaining backward compatibility. """ +import logging from typing import Any, TypedDict from comfy.cli_args import args @@ -27,11 +28,6 @@ CLI_FEATURE_FLAG_REGISTRY: dict[str, FeatureFlagInfo] = { } -def get_cli_feature_flag_registry() -> dict[str, FeatureFlagInfo]: - """Return the registry of known CLI-settable feature flags.""" - return {k: dict(v) for k, v in CLI_FEATURE_FLAG_REGISTRY.items()} - - _COERCE_FNS: dict[str, Any] = { "bool": lambda v: v.lower() == "true", "int": lambda v: int(v), @@ -40,26 +36,41 @@ _COERCE_FNS: dict[str, Any] = { def _coerce_flag_value(key: str, raw_value: str) -> Any: - """Coerce a raw string value using the registry type, or keep as string.""" + """Coerce a raw string value using the registry type, or keep as string. + + Returns the raw string if the key is unregistered, the type is unknown, + or coercion fails (with a warning logged in the failure case). + """ 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) + try: + return coerce(raw_value) + except (ValueError, TypeError): + logging.warning( + "Could not coerce --feature-flag %s=%r to %s; using raw string.", + key, raw_value, info["type"], + ) + return raw_value def _parse_cli_feature_flags() -> dict[str, Any]: - """Parse --feature-flag key=value pairs from CLI args into a dict.""" + """Parse --feature-flag key=value pairs from CLI args into a dict. + + Items without '=' default to the value 'true' (bare flag form). + """ result: dict[str, Any] = {} for item in getattr(args, "feature_flag", []): - if "=" not in item: - continue - key, _, raw_value = item.partition("=") + key, sep, raw_value = item.partition("=") key = key.strip() - if key: - result[key] = _coerce_flag_value(key, raw_value.strip()) + if not key: + continue + if not sep: + raw_value = "true" + result[key] = _coerce_flag_value(key, raw_value.strip()) return result diff --git a/main.py b/main.py index 8bc219cb0..a6fdaf43c 100644 --- a/main.py +++ b/main.py @@ -5,8 +5,8 @@ from comfy.cli_args import args if args.list_feature_flags: import json - from comfy_api.feature_flags import get_cli_feature_flag_registry - print(json.dumps(get_cli_feature_flag_registry(), indent=2)) # noqa: T201 + 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 diff --git a/tests-unit/feature_flags_test.py b/tests-unit/feature_flags_test.py index 34f14818e..26add1cfd 100644 --- a/tests-unit/feature_flags_test.py +++ b/tests-unit/feature_flags_test.py @@ -4,7 +4,7 @@ from comfy_api.feature_flags import ( get_connection_feature, supports_feature, get_server_features, - get_cli_feature_flag_registry, + CLI_FEATURE_FLAG_REGISTRY, SERVER_FEATURE_FLAGS, _coerce_flag_value, _parse_cli_feature_flags, @@ -116,6 +116,15 @@ class TestCoerceFlagValue: assert _coerce_flag_value("unknown_flag", "true") == "true" assert _coerce_flag_value("unknown_flag", "42") == "42" + def test_failed_coercion_falls_back_to_string(self, monkeypatch): + """Malformed values for typed flags must not crash; raw string is returned.""" + monkeypatch.setitem( + CLI_FEATURE_FLAG_REGISTRY, + "test_int_flag", + {"type": "int", "default": 0, "description": "test"}, + ) + assert _coerce_flag_value("test_int_flag", "not_a_number") == "not_a_number" + class TestParseCliFeatureFlags: """Test suite for _parse_cli_feature_flags.""" @@ -125,8 +134,14 @@ class TestParseCliFeatureFlags: result = _parse_cli_feature_flags() assert result == {"show_signin_button": True} - def test_missing_equals_skipped(self, monkeypatch): - monkeypatch.setattr("comfy_api.feature_flags.args", type("Args", (), {"feature_flag": ["noequals", "valid=1"]})()) + 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"} @@ -135,7 +150,7 @@ class TestCliFeatureFlagRegistry: """Test suite for the CLI feature flag registry.""" def test_registry_entries_have_required_fields(self): - for key, info in get_cli_feature_flag_registry().items(): + 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'"