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'"