fix(config): normalize nullable arrays and objects instead of refusing them (#642)

* fix(config): normalize nullable arrays and objects instead of refusing them

`element_style._nullable` widens every `customization.modes.<mode>` override
with 'null' so a blank means "inherit the base", which turns a colour declared
"array" into ["array", "null"]. `normalize_config_values` only knew how to
convert null/integer/number/boolean out of a union, so a valid [0, 249, 0]
matched nothing and logged

    Could not normalize field customization.modes.upcoming.odds_text.text_color:
    value=[0, 249, 0], type=<class 'list'>, schema_type=['array', 'null']

The warning was the harmless half. It `continue`d past the single-type handling
below, where `prop_type == 'array'` coerces items, so a nullable array never had
its items normalized while a plain one did. A form posts numbers as strings, so
["0", "249", "0"] survived to the validator and was rejected with "Expected type
integer, got str" -- setting a per-mode colour in the web UI failed outright.
Every per-mode override of a structural or string type was exposed, across all
eight scoreboard plugins, not only colours.

Re-enter the single-type handling with the matched member rather than bailing,
accept a string that matches, and warn only on a genuine mismatch so the
diagnostic still reaches the validator.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ET8e5weDrb5Ju5QTKLU7zh

* fix(config): convert only integral numbers for integer array items

Review catch on the previous commit. Routing nullable arrays into the shared
item handling made its integer coercion reachable for them, and that coercion
called int(v) on any number: a client sending [2.5, 249, 0] for an RGB array
got 2 stored and a 200 back, so a wrong value was silently corrected into a
valid-looking one rather than refused.

Convert only genuinely integral values, at both the union-item and the plain
'array' item branch so the two cannot drift. A whole float -- 2.0, which is all
JSON can express for an integer -- still converts. This also settles an
inconsistency that predates the change: int('2.5') raises, so the string form
was always preserved and rejected while the numeric form was truncated.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ET8e5weDrb5Ju5QTKLU7zh

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

test/web_interface/test_normalize_nullable_structural_types.py

@@ -0,0 +1,164 @@
+"""Tests for the union-type branch of normalize_config_values, which the
+plugin-config save path (_prepare_plugin_config_for_save in api_v3/plugins.py)
+runs before validation.
+
+``customization.modes.<mode>`` overrides are generated by
+``element_style._nullable``, which widens every field's declared type with
+``'null'`` so a blank means "inherit the base". That turns a colour declared
+``"array"`` into ``["array", "null"]``. The union branch only knew how to
+convert null/integer/number/boolean, so a valid ``[0, 249, 0]`` matched nothing:
+it logged "Could not normalize field ..." and, because it then skipped the
+single-type handling, never normalized the array's items either.
+"""
+
+import logging
+
+import pytest
+
+from web_interface.blueprints.api_v3.plugins import (
+    _prepare_plugin_config_for_save,
+)
+
+
+class _SchemaMgr:
+    """Minimal stand-in for the real schema manager.
+
+    Defaults are empty and validation always passes, so these tests isolate the
+    normalization pass rather than re-testing the validator.
+    """
+
+    def generate_default_config(self, plugin_id, use_cache=True):
+        return {}
+
+    def validate_config_against_schema(self, config, schema, plugin_id):
+        return True, []
+
+
+def _rgb_prop(nullable):
+    """An RGB colour field as the base schema (plain) or a mode override."""
+    prop = {"type": "array", "items": {"type": "integer"},
+            "minItems": 3, "maxItems": 3}
+    if nullable:
+        prop = dict(prop, type=["array", "null"], default=None)
+    return prop
+
+
+def _schema(nullable):
+    """A scoreboard-shaped schema: customization.modes.upcoming.odds_text."""
+    return {
+        "type": "object",
+        "properties": {
+            "enabled": {"type": "boolean"},
+            "customization": {
+                "type": "object",
+                "properties": {
+                    "modes": {
+                        "type": "object",
+                        "properties": {
+                            "upcoming": {
+                                "type": "object",
+                                "properties": {
+                                    "odds_text": {
+                                        "type": "object",
+                                        "properties": {
+                                            "text_color": _rgb_prop(nullable),
+                                        },
+                                    },
+                                },
+                            },
+                        },
+                    },
+                },
+            },
+        },
+    }
+
+
+def _save(config, nullable=True):
+    regular, _secrets, error = _prepare_plugin_config_for_save(
+        "baseball-scoreboard", config, _schema(nullable), _SchemaMgr(),
+        is_json=True,
+    )
+    assert error is None
+    return regular
+
+
+def _colour(config):
+    return (config["customization"]["modes"]["upcoming"]
+            ["odds_text"]["text_color"])
+
+
+def _submit(value):
+    return {"enabled": True,
+            "customization": {"modes": {"upcoming": {
+                "odds_text": {"text_color": value}}}}}
+
+
+class TestNullableArrayOverride:
+    """['array', 'null'] holding a real list is valid, not a failure."""
+
+    def test_valid_rgb_survives_the_save(self) -> None:
+        """The exact value from the bug report is stored unchanged."""
+        assert _colour(_save(_submit([0, 249, 0]))) == [0, 249, 0]
+
+    def test_valid_rgb_does_not_warn(self, caplog) -> None:
+        """No "Could not normalize" for a value that already matches."""
+        with caplog.at_level(logging.WARNING):
+            _save(_submit([0, 249, 0]))
+        assert "Could not normalize field" not in caplog.text
+
+    def test_items_are_normalized_like_a_plain_array(self) -> None:
+        """The real defect: form data posts strings, and a nullable array used
+        to keep them while a plain 'array' turned them into ints."""
+        nullable = _colour(_save(_submit(["0", "249", "0"])))
+        plain = _colour(_save(_submit(["0", "249", "0"]), nullable=False))
+        assert nullable == [0, 249, 0]
+        assert nullable == plain
+
+    def test_null_still_means_inherit(self) -> None:
+        """Widening the type must not break the blank-means-inherit case."""
+        assert _colour(_save(_submit(None))) is None
+
+    @pytest.mark.parametrize("blank", ["", "  ", "null", "None"])
+    def test_blank_strings_still_become_null(self, blank) -> None:
+        """A cleared form field arrives as a string and still means inherit."""
+        assert _colour(_save(_submit(blank))) is None
+
+
+class TestFractionalItemsReachTheValidator:
+    """int(2.5) == 2 would store a corrected number where the client sent a
+    wrong one, so the save looks successful and the config is quietly wrong.
+    Routing nullable arrays into the shared item handling made this reachable
+    for them too, so both paths convert only genuinely integral values."""
+
+    @pytest.mark.parametrize("nullable", [True, False])
+    def test_a_fractional_item_is_not_truncated(self, nullable) -> None:
+        """2.5 reaches the validator intact, which then refuses it for an
+        integer item. Storing 2 would make the save look successful."""
+        assert _colour(_save(_submit([2.5, 249, 0]),
+                             nullable=nullable)) == [2.5, 249, 0]
+
+    @pytest.mark.parametrize("nullable", [True, False])
+    def test_a_whole_float_still_converts(self, nullable) -> None:
+        """JSON has no int/float distinction, so 2.0 is a legitimate integer."""
+        assert _colour(_save(_submit([2.0, 249.0, 0.0]),
+                             nullable=nullable)) == [2, 249, 0]
+
+    @pytest.mark.parametrize("nullable", [True, False])
+    def test_a_fractional_string_is_preserved_too(self, nullable) -> None:
+        """int('2.5') raises, so this path always preserved the value --
+        pinned so the numeric and string cases cannot drift apart again."""
+        assert _colour(_save(_submit(["2.5", "249", "0"]),
+                             nullable=nullable)) == ["2.5", 249, 0]
+
+
+class TestUnionFallbackStillReports:
+    """A union the value genuinely does not match must still warn."""
+
+    def test_unmatched_value_warns_and_passes_through(self, caplog) -> None:
+        """An object where ['array', 'null'] is declared is a real mismatch,
+        so it keeps reaching the validator with the diagnostic intact."""
+        with caplog.at_level(logging.WARNING):
+            saved = _save(_submit({"r": 0}))
+        assert "Could not normalize field" in caplog.text
+        assert _colour(saved) == {"r": 0}

web_interface/blueprints/api_v3/plugins.py

@@ -2080,10 +2080,29 @@ def _prepare_plugin_config_for_save(plugin_id, plugin_config, schema, schema_mgr
                         normalized[key] = value.strip().lower() in ('true', '1', 'on', 'yes')
                         continue
 
-                # Nothing converted: keep the value for validation to report.
+                # The scalar conversions above are the only ones this branch
-                logger.warning(f"Could not normalize field {field_path}: value={repr(value)}, type={type(value)}, schema_type={prop_type}")
+                # knows, so a union naming a structural or string type fell
-                normalized[key] = value
+                # through here even when the value already matched it --
-                continue
+                # customization.modes.<mode> makes every override nullable
+                # (see element_style._nullable), so every per-mode colour is
+                # ['array', 'null'] and warned on a perfectly valid [r, g, b].
+                # Worse than the noise: `continue` skipped the single-type
+                # handling below, so a nullable array never had its items
+                # normalized and form-posted ["0", "249", "0"] stayed strings
+                # where a plain 'array' field would have become ints. Re-enter
+                # that handling with the matched member instead.
+                if isinstance(value, list) and 'array' in prop_type:
+                    prop_type = 'array'
+                elif isinstance(value, dict) and 'object' in prop_type:
+                    prop_type = 'object'
+                elif isinstance(value, str) and 'string' in prop_type:
+                    normalized[key] = value
+                    continue
+                else:
+                    # Nothing converted: keep the value for validation to report.
+                    logger.warning(f"Could not normalize field {field_path}: value={repr(value)}, type={type(value)}, schema_type={prop_type}")
+                    normalized[key] = value
+                    continue
 
             if isinstance(value, dict) and prop_type == 'object' and 'properties' in prop_schema:
                 # Recursively normalize nested objects
@@ -2112,7 +2131,16 @@ def _prepare_plugin_config_for_save(plugin_id, plugin_config, schema, schema_mgr
                                 except (ValueError, TypeError, OverflowError):
                                     pass
                             elif isinstance(v, (int, float)):
-                                normalized_array.append(int(v))
+                                # Only a genuinely integral value converts.
+                                # int(2.5) == 2 would store a silently
+                                # corrected number where the client sent a
+                                # wrong one; leaving it lets the validator
+                                # reject it. A whole float (2.0 out of JSON)
+                                # is integral and still converts.
+                                if isinstance(v, int) or float(v).is_integer():
+                                    normalized_array.append(int(v))
+                                else:
+                                    normalized_array.append(v)
                                 continue
                         elif 'number' in item_type:
                             if isinstance(v, str):
@@ -2138,7 +2166,11 @@ def _prepare_plugin_config_for_save(plugin_id, plugin_config, schema, schema_mgr
                             except (ValueError, TypeError, OverflowError):
                                 normalized_array.append(v)
                         elif isinstance(v, (int, float)):
-                            normalized_array.append(int(v))
+                            # Integral only -- see the union branch above.
+                            if isinstance(v, int) or float(v).is_integer():
+                                normalized_array.append(int(v))
+                            else:
+                                normalized_array.append(v)
                         else:
                             normalized_array.append(v)
                     normalized[key] = normalized_array