diff --git a/gateway/config.py b/gateway/config.py index ed59c43ff632..cc80cda40155 100644 --- a/gateway/config.py +++ b/gateway/config.py @@ -74,6 +74,23 @@ def _env_multiplex_profiles_override() -> "bool | None": return None +def _normalize_transport_token(value: Any) -> str: + """Normalize a streaming transport/mode value to a canonical token. + + Handles the YAML 1.1 boolean quirk where bare ``on`` / ``off`` parse to + Python ``True`` / ``False`` (see ``gateway/display_config.py`` ``_normalise``). + Without this, ``mode: off`` arrives as boolean ``False`` and stringifying it + yields ``"false"`` instead of the advertised ``"off"``, so streaming would be + enabled instead of disabled. Booleans map to ``"auto"`` (True) / ``"off"`` + (False); anything else is lower-cased, defaulting to ``"auto"``. + """ + if value is None: + return "auto" + if isinstance(value, bool): + return "auto" if value else "off" + return str(value).strip().lower() or "auto" + + def _coerce_float(value: Any, default: float) -> float: """Coerce numeric config values, falling back on malformed input.""" if value is None: @@ -727,19 +744,25 @@ class StreamingConfig: # a surprising footgun: the whole reply buffers and sends at once. # ``mode: off`` disables streaming; an explicit ``enabled`` key always # wins so callers can force either state. + # + # ``transport`` alone does NOT imply ``enabled``: ``streaming.enabled`` + # is the documented master switch (see website/docs/user-guide/ + # configuration.md), so a bare ``transport`` only selects HOW to stream + # once streaming is on. Only the ``mode`` alias flips ``enabled``. raw_transport = data.get("transport") raw_mode = data.get("mode") - transport = raw_transport if raw_transport is not None else raw_mode - if transport is None: - transport = "auto" - transport = str(transport).strip().lower() or "auto" + # Normalize both through the same helper so YAML's bare ``off``/``on`` + # (parsed as bool False/True) become canonical tokens rather than + # ``"false"``/``"true"``. + picked = raw_transport if raw_transport is not None else raw_mode + transport = _normalize_transport_token(picked) if "enabled" in data: enabled = _coerce_bool(data.get("enabled"), False) - elif raw_mode is not None or raw_transport is not None: - # A mode/transport was given without an explicit enabled flag: - # infer enabled from the transport ("off" = disabled, else on). - enabled = transport != "off" + elif raw_mode is not None: + # The ``mode`` alias (and only ``mode``) infers enabled: + # ``off`` disables, anything else enables. + enabled = _normalize_transport_token(raw_mode) != "off" else: enabled = False diff --git a/tests/test_gateway_streaming_nested_config.py b/tests/test_gateway_streaming_nested_config.py index cead58fd4625..cd49079787ac 100644 --- a/tests/test_gateway_streaming_nested_config.py +++ b/tests/test_gateway_streaming_nested_config.py @@ -90,12 +90,17 @@ class TestStreamingModeAlias: # transport still resolves from mode assert sc.transport == "auto" - def test_transport_takes_precedence_over_mode(self): + def test_transport_selects_transport_but_mode_controls_enabled(self): + """``transport`` picks HOW to stream; ``mode`` is the enable alias. + + With ``mode: off`` the master switch is off even though ``transport`` + selects the draft path — ``enabled`` is inferred from ``mode`` only. + """ from gateway.config import StreamingConfig sc = StreamingConfig.from_dict({"mode": "off", "transport": "draft"}) assert sc.transport == "draft" - assert sc.enabled is True + assert sc.enabled is False def test_empty_block_stays_disabled(self): from gateway.config import StreamingConfig @@ -103,9 +108,59 @@ class TestStreamingModeAlias: sc = StreamingConfig.from_dict({}) assert sc.enabled is False - def test_transport_only_enables(self): + def test_transport_only_does_not_enable(self): + """``streaming.enabled`` is the documented master switch, so a bare + ``transport`` must not flip streaming on by itself.""" from gateway.config import StreamingConfig sc = StreamingConfig.from_dict({"transport": "edit"}) - assert sc.enabled is True + assert sc.enabled is False + # transport is still recorded so it takes effect once streaming is on. assert sc.transport == "edit" + + +class TestStreamingYamlBooleanQuirk: + """YAML 1.1 parses bare ``off``/``on`` as booleans; ``mode``/``transport`` + must normalize those back to canonical string tokens. + + Regression for the review on PR #62873: bare ``mode: off`` arrived as + Python ``False`` and stringified to ``"false"``, which is not ``"off"``, + so streaming was enabled instead of honoring the advertised disable. + """ + + def test_mode_bare_off_boolean_disables(self): + from gateway.config import StreamingConfig + + # yaml.safe_load("off") -> False + sc = StreamingConfig.from_dict({"mode": False}) + assert sc.enabled is False + assert sc.transport == "off" + + def test_mode_bare_on_boolean_enables(self): + from gateway.config import StreamingConfig + + # yaml.safe_load("on") -> True + sc = StreamingConfig.from_dict({"mode": True}) + assert sc.enabled is True + assert sc.transport == "auto" + + def test_transport_bare_off_boolean_normalizes(self): + from gateway.config import StreamingConfig + + # transport alone never enables, but the token must still normalize. + sc = StreamingConfig.from_dict({"transport": False}) + assert sc.enabled is False + assert sc.transport == "off" + + def test_loader_normalizes_bare_yaml_off(self): + """End-to-end through load_gateway_config(): unquoted ``mode: off`` + (a YAML boolean) must keep streaming disabled.""" + cfg = _load_with_yaml_dict({"streaming": {"mode": False}}) + assert cfg.streaming.enabled is False + assert cfg.streaming.transport == "off" + + def test_loader_nested_mode_enables(self): + """Nested gateway.streaming.mode alias enables through the loader.""" + cfg = _load_with_yaml_dict({"gateway": {"streaming": {"mode": "edit"}}}) + assert cfg.streaming.enabled is True + assert cfg.streaming.transport == "edit"