diff --git a/gateway/display_config.py b/gateway/display_config.py index b95086aa61..db6907734c 100644 --- a/gateway/display_config.py +++ b/gateway/display_config.py @@ -84,22 +84,44 @@ def resolve_display_setting(user_config: dict, platform_key: str, setting: str, ``platform_key`` is the platform config key (``"telegram"``; see ``_platform_config_key`` in gateway/run.py). Returns *fallback* when nothing is configured. """ - display_cfg = user_config.get("display") or {} - plat_overrides = (display_cfg.get("platforms") or {}).get(platform_key) - if isinstance(plat_overrides, dict) and plat_overrides.get(setting) is not None: - return _normalise(setting, plat_overrides[setting]) - if setting == "tool_progress": # legacy display.tool_progress_overrides. - legacy = display_cfg.get("tool_progress_overrides") - if isinstance(legacy, dict) and legacy.get(platform_key) is not None: - return _normalise(setting, legacy[platform_key]) - if setting != "streaming" and display_cfg.get(setting) is not None: # display.streaming is CLI-only - return _normalise(setting, display_cfg[setting]) + configured = _configured_display_value(user_config, platform_key, setting) + if configured is not None: + return _normalise(setting, configured) val = _PLATFORM_DEFAULTS.get(platform_key, {}).get(setting) if val is None: val = _GLOBAL_DEFAULTS.get(setting) return fallback if val is None else val +def _configured_display_value(user_config: dict, platform_key: str, setting: str) -> Any: + """First non-None operator value, without introducing tier defaults.""" + display_cfg = user_config.get("display") or {} + plat_overrides = (display_cfg.get("platforms") or {}).get(platform_key) + if isinstance(plat_overrides, dict) and plat_overrides.get(setting) is not None: + return plat_overrides[setting] + if setting == "tool_progress": + legacy = display_cfg.get("tool_progress_overrides") + if isinstance(legacy, dict) and legacy.get(platform_key) is not None: + return legacy[platform_key] + if setting != "streaming": # display.streaming is CLI-only + return display_cfg.get(setting) + return None + + +def resolve_tool_progress(user_config: dict, platform_key: str, env_mode: str | None = None) -> tuple[str, bool]: + """Return (mode, explicit intent) from the same winning source. + + Non-None YAML wins over the legacy env bridge. Null inherits through to env, + then tier defaults. A tier's off is not an operator request to disable cards. + """ + configured = _configured_display_value(user_config, platform_key, "tool_progress") + if configured is not None: + return _normalise("tool_progress", configured), True + if env_mode: + return _normalise("tool_progress", env_mode), True + return resolve_display_setting(user_config, platform_key, "tool_progress"), False + + # --- Normalisation of YAML quirks (bare ``off`` → False in YAML 1.1, etc.) --- _TRUTHY = {"true", "1", "yes", "on"} diff --git a/gateway/run_turn.py b/gateway/run_turn.py index 1fc77129ca..521d741924 100644 --- a/gateway/run_turn.py +++ b/gateway/run_turn.py @@ -2710,11 +2710,6 @@ class GatewayTurnMixin: platform_key = _platform_config_key(source.platform) enabled_toolsets, disabled_toolsets = self._resolve_turn_toolsets(user_config, source, platform_key) adapter = self._adapter_for_source(source) - # display.platforms.. → display. → built-in platform defaults. - _display_cfg = user_config.get("display", {}) - if not isinstance(_display_cfg, dict): - _display_cfg = {} - # Tool preview length (0 = no limit) and friendly tool labels (default on), per-platform. for _setter, _setting, _default, _cast in ( ("set_tool_preview_max_len", "tool_preview_length", 0, lambda v: int(v) if v else 0), @@ -2725,24 +2720,10 @@ class GatewayTurnMixin: _val = resolve_display_setting(user_config, platform_key, _setting, _default) getattr(_agent_display, _setter)(_cast(_val)) - # Tool progress mode; HERMES_TOOL_PROGRESS_MODE wins only when the config never set it. - _resolved_tp = resolve_display_setting(user_config, platform_key, "tool_progress") - _env_tp = os.getenv("HERMES_TOOL_PROGRESS_MODE") - _platform_cfg = (_display_cfg.get("platforms") or {}).get(platform_key) or {} - _legacy_tp_overrides = _display_cfg.get("tool_progress_overrides") or {} - _tool_progress_configured = "tool_progress" in _display_cfg or any( - isinstance(cfg, dict) and key in cfg - for cfg, key in ((_platform_cfg, "tool_progress"), (_legacy_tp_overrides, platform_key)) - ) - progress_mode = _env_tp if _env_tp and not _tool_progress_configured else (_resolved_tp or _env_tp or "all") - # Operator intent vs tier default: True when a human WROTE a mode (config or env). A ``null`` - # value is inheritance, not intent — the resolver skips None (display_config.py), so a bare - # key with no value must not read as "explicitly off". - _tool_progress_explicit = bool(_env_tp) or any( - isinstance(cfg, dict) and cfg.get(key) is not None - for cfg, key in ( - (_display_cfg, "tool_progress"), (_platform_cfg, "tool_progress"), (_legacy_tp_overrides, platform_key), - ) + # Resolve the mode and its provenance together: null inherits, tier off is not intent. + from gateway.display_config import resolve_tool_progress + progress_mode, _tool_progress_explicit = resolve_tool_progress( + user_config, platform_key, os.getenv("HERMES_TOOL_PROGRESS_MODE"), ) # "accumulate" (edit one bubble) or "separate" (one msg per tool) progress_grouping = resolve_display_setting(user_config, platform_key, "tool_progress_grouping") or "accumulate" diff --git a/tests/gateway/test_display_config.py b/tests/gateway/test_display_config.py index 3a74058978..62ca85c1fe 100644 --- a/tests/gateway/test_display_config.py +++ b/tests/gateway/test_display_config.py @@ -5,6 +5,25 @@ # Resolver: resolution order # --------------------------------------------------------------------------- +class TestToolProgressProvenance: + def test_winning_source_controls_mode_and_intent(self): + from gateway.display_config import resolve_tool_progress + + cases = [ + ({}, None, ("off", False)), + ({}, "all", ("all", True)), + ({"tool_progress": None}, "all", ("all", True)), + ({"platforms": {"slack": {"tool_progress": None}}}, "off", ("off", True)), + ({"tool_progress_overrides": {"slack": None}}, "new", ("new", True)), + ({"tool_progress": False}, "all", ("off", True)), + ({"tool_progress": "all", "platforms": {"slack": {"tool_progress": None}}}, "off", ("all", True)), + ({"tool_progress": "off", "tool_progress_overrides": {"slack": "new"}}, "all", ("new", True)), + ({"tool_progress_overrides": {"slack": "off"}, "platforms": {"slack": {"tool_progress": "all"}}}, None, ("all", True)), + ] + for display, env, expected in cases: + assert resolve_tool_progress({"display": display}, "slack", env) == expected + + class TestResolveDisplaySetting: """resolve_display_setting() resolves with correct priority.""" diff --git a/tests/gateway/test_run_progress_topics.py b/tests/gateway/test_run_progress_topics.py index 5591d16f57..36ae83fbdc 100644 --- a/tests/gateway/test_run_progress_topics.py +++ b/tests/gateway/test_run_progress_topics.py @@ -1187,8 +1187,12 @@ async def test_slack_operator_tool_progress_off_disables_task_cards(monkeypatch, ], ids=["platform-null", "global-null", "legacy-null", "platform-null-over-global-all"], ) -async def test_slack_null_tool_progress_is_inheritance_not_explicit_off(monkeypatch, tmp_path, display_cfg): +@pytest.mark.parametrize("env_mode", [None, "all", "new"]) +async def test_slack_null_tool_progress_is_inheritance_not_explicit_off(monkeypatch, tmp_path, display_cfg, env_mode): # A bare key with ``null`` inherits (the resolver skips None); it is not an operator saying "off". + monkeypatch.delenv("HERMES_TOOL_PROGRESS_MODE", raising=False) + if env_mode is not None: + monkeypatch.setenv("HERMES_TOOL_PROGRESS_MODE", env_mode) adapter, result = await _run_with_agent( monkeypatch, tmp_path,