From bfa60234c8bb855aec7ae986eef3767ddadcf3a6 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Fri, 24 Apr 2026 12:49:02 -0500 Subject: [PATCH] feat(tui): warn on bare null sections in config.yaml MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tolerating null top-level keys silently drops user settings (e.g. `agent.system_prompt` next to a bare `agent:` line is gone). Probe at session create, log via `logger.warning`, and surface in the boot info under `config_warning` — rendered in the TUI feed alongside the existing `credential_warning` banner. --- tests/tui_gateway/test_make_agent_provider.py | 49 +++++++--- tui_gateway/server.py | 89 +++++++++++++------ ui-tui/src/app/useSessionLifecycle.ts | 4 + ui-tui/src/gatewayTypes.ts | 2 +- 4 files changed, 106 insertions(+), 38 deletions(-) diff --git a/tests/tui_gateway/test_make_agent_provider.py b/tests/tui_gateway/test_make_agent_provider.py index 3e899be32..7ad900d0d 100644 --- a/tests/tui_gateway/test_make_agent_provider.py +++ b/tests/tui_gateway/test_make_agent_provider.py @@ -27,16 +27,22 @@ def test_make_agent_passes_resolved_provider(): "agent": {"system_prompt": "test"}, } - with patch("tui_gateway.server._load_cfg", return_value=fake_cfg), \ - patch("tui_gateway.server._get_db", return_value=MagicMock()), \ - patch("tui_gateway.server._load_tool_progress_mode", return_value="compact"), \ - patch("tui_gateway.server._load_reasoning_config", return_value=None), \ - patch("tui_gateway.server._load_service_tier", return_value=None), \ - patch("tui_gateway.server._load_enabled_toolsets", return_value=None), \ - patch("hermes_cli.runtime_provider.resolve_runtime_provider", return_value=fake_runtime) as mock_resolve, \ - patch("run_agent.AIAgent") as mock_agent: + with ( + patch("tui_gateway.server._load_cfg", return_value=fake_cfg), + patch("tui_gateway.server._get_db", return_value=MagicMock()), + patch("tui_gateway.server._load_tool_progress_mode", return_value="compact"), + patch("tui_gateway.server._load_reasoning_config", return_value=None), + patch("tui_gateway.server._load_service_tier", return_value=None), + patch("tui_gateway.server._load_enabled_toolsets", return_value=None), + patch( + "hermes_cli.runtime_provider.resolve_runtime_provider", + return_value=fake_runtime, + ) as mock_resolve, + patch("run_agent.AIAgent") as mock_agent, + ): from tui_gateway.server import _make_agent + _make_agent("sid-1", "key-1") mock_resolve.assert_called_once_with(requested=None) @@ -48,6 +54,19 @@ def test_make_agent_passes_resolved_provider(): assert call_kwargs.kwargs["api_mode"] == "anthropic_messages" +def test_probe_config_health_flags_null_sections(): + """Bare YAML keys (`agent:` with no value) parse as None and silently + drop nested settings; probe must surface them so users can fix.""" + from tui_gateway.server import _probe_config_health + + assert _probe_config_health({"agent": {"x": 1}}) == "" + assert _probe_config_health({}) == "" + + msg = _probe_config_health({"agent": None, "display": None, "model": {}}) + assert "agent" in msg and "display" in msg + assert "model" not in msg + + def test_make_agent_tolerates_null_config_sections(): """Bare `agent:` / `display:` keys in ~/.hermes/config.yaml parse as None. cfg.get("agent", {}) returns None (default only fires on missing @@ -65,12 +84,18 @@ def test_make_agent_tolerates_null_config_sections(): } null_cfg = {"agent": None, "display": None, "model": {"default": "glm-5"}} - with patch("tui_gateway.server._load_cfg", return_value=null_cfg), \ - patch("tui_gateway.server._get_db", return_value=MagicMock()), \ - patch("hermes_cli.runtime_provider.resolve_runtime_provider", return_value=fake_runtime), \ - patch("run_agent.AIAgent") as mock_agent: + with ( + patch("tui_gateway.server._load_cfg", return_value=null_cfg), + patch("tui_gateway.server._get_db", return_value=MagicMock()), + patch( + "hermes_cli.runtime_provider.resolve_runtime_provider", + return_value=fake_runtime, + ), + patch("run_agent.AIAgent") as mock_agent, + ): from tui_gateway.server import _make_agent + _make_agent("sid-null", "key-null") assert mock_agent.called diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 4dbb8318a..a854a84f2 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -61,7 +61,11 @@ def _panic_hook(exc_type, exc_value, exc_tb): # Stderr goes through to the TUI as a gateway.stderr Activity line — # the first line here is what the user will see without opening any # log files. Rest of the stack is still in the log for full context. - first = str(exc_value).strip().splitlines()[0] if str(exc_value).strip() else exc_type.__name__ + first = ( + str(exc_value).strip().splitlines()[0] + if str(exc_value).strip() + else exc_type.__name__ + ) print(f"[gateway-crash] {exc_type.__name__}: {first}", file=sys.stderr, flush=True) # Chain to the default hook so the process still terminates normally. sys.__excepthook__(exc_type, exc_value, exc_tb) @@ -593,13 +597,17 @@ def _coerce_statusbar(raw) -> str: def _load_reasoning_config() -> dict | None: from hermes_constants import parse_reasoning_effort - effort = str((_load_cfg().get("agent") or {}).get("reasoning_effort", "") or "").strip() + effort = str( + (_load_cfg().get("agent") or {}).get("reasoning_effort", "") or "" + ).strip() return parse_reasoning_effort(effort) def _load_service_tier() -> str | None: raw = ( - str((_load_cfg().get("agent") or {}).get("service_tier", "") or "").strip().lower() + str((_load_cfg().get("agent") or {}).get("service_tier", "") or "") + .strip() + .lower() ) if not raw or raw in {"normal", "default", "standard", "off", "none"}: return None @@ -816,6 +824,22 @@ def _probe_credentials(agent) -> str: return "" +def _probe_config_health(cfg: dict) -> str: + """Flag bare YAML keys (`agent:` with no value → None) that silently + drop nested settings. Returns warning or ''.""" + if not isinstance(cfg, dict): + return "" + null_keys = sorted(k for k, v in cfg.items() if v is None) + if not null_keys: + return "" + keys = ", ".join(f"`{k}`" for k in null_keys) + return ( + f"config.yaml has empty section(s): {keys}. " + f"Remove the line(s) or set them to `{{}}` — " + f"empty sections silently drop nested settings." + ) + + def _session_info(agent) -> dict: info: dict = { "model": getattr(agent, "model", ""), @@ -1115,7 +1139,9 @@ def _resolve_personality_prompt(cfg: dict) -> str: try: from hermes_cli.config import load_config as _load_full_cfg - personalities = (_load_full_cfg().get("agent") or {}).get("personalities", {}) + personalities = (_load_full_cfg().get("agent") or {}).get( + "personalities", {} + ) except Exception: personalities = (cfg.get("agent") or {}).get("personalities", {}) pval = personalities.get(name) @@ -1503,6 +1529,10 @@ def _(rid, params: dict) -> dict: warn = _probe_credentials(agent) if warn: info["credential_warning"] = warn + cfg_warn = _probe_config_health(_load_cfg()) + if cfg_warn: + info["config_warning"] = cfg_warn + logger.warning(cfg_warn) _emit("session.info", sid, info) except Exception as e: session["agent_error"] = str(e) @@ -1649,9 +1679,7 @@ def _(rid, params: dict) -> dict: return _db_unavailable_error(rid, code=5007) title, key = params.get("title", ""), session["session_key"] if not title: - return _ok( - rid, {"title": db.get_session_title(key) or "", "session_key": key} - ) + return _ok(rid, {"title": db.get_session_title(key) or "", "session_key": key}) try: db.set_session_title(key, title) return _ok(rid, {"title": title}) @@ -2278,7 +2306,9 @@ def _(rid, params: dict) -> dict: f.write(trace) except Exception: pass - print(f"[gateway-turn] {type(e).__name__}: {e}", file=sys.stderr, flush=True) + print( + f"[gateway-turn] {type(e).__name__}: {e}", file=sys.stderr, flush=True + ) _emit("error", sid, {"message": str(e)}) finally: try: @@ -2701,9 +2731,7 @@ def _(rid, params: dict) -> dict: cfg = _load_cfg() display = cfg.get("display") if isinstance(cfg.get("display"), dict) else {} sections_cfg = ( - display.get("sections") - if isinstance(display.get("sections"), dict) - else {} + display.get("sections") if isinstance(display.get("sections"), dict) else {} ) nv = str(value or "").strip().lower() @@ -2842,11 +2870,14 @@ def _(rid, params: dict) -> dict: ) if key == "personality": return _ok( - rid, {"value": (_load_cfg().get("display") or {}).get("personality", "default")} + rid, + {"value": (_load_cfg().get("display") or {}).get("personality", "default")}, ) if key == "reasoning": cfg = _load_cfg() - effort = str((cfg.get("agent") or {}).get("reasoning_effort", "medium") or "medium") + effort = str( + (cfg.get("agent") or {}).get("reasoning_effort", "medium") or "medium" + ) display = ( "show" if bool((cfg.get("display") or {}).get("show_reasoning", False)) @@ -2868,7 +2899,11 @@ def _(rid, params: dict) -> dict: if key == "thinking_mode": allowed_tm = frozenset({"collapsed", "truncated", "full"}) cfg = _load_cfg() - raw = str((cfg.get("display") or {}).get("thinking_mode", "") or "").strip().lower() + raw = ( + str((cfg.get("display") or {}).get("thinking_mode", "") or "") + .strip() + .lower() + ) if raw in allowed_tm: nv = raw else: @@ -3369,7 +3404,16 @@ def _list_repo_files(root: str) -> list[str]: if top_result.returncode == 0: top = top_result.stdout.decode("utf-8", "replace").strip() list_result = subprocess.run( - ["git", "-C", top, "ls-files", "-z", "--cached", "--others", "--exclude-standard"], + [ + "git", + "-C", + top, + "ls-files", + "-z", + "--cached", + "--others", + "--exclude-standard", + ], capture_output=True, timeout=2.0, check=False, @@ -3378,7 +3422,9 @@ def _list_repo_files(root: str) -> list[str]: for p in list_result.stdout.decode("utf-8", "replace").split("\0"): if not p: continue - rel = os.path.relpath(os.path.join(top, p), root).replace(os.sep, "/") + rel = os.path.relpath(os.path.join(top, p), root).replace( + os.sep, "/" + ) # Skip parents/siblings of cwd — keep the picker scoped # to root-and-below, matching Cmd-P workspace semantics. if rel.startswith("../"): @@ -3512,12 +3558,7 @@ def _(rid, params: dict) -> dict: # editors like Cursor / VS Code do for Cmd-P. Path-ish queries (with # `/`, `./`, `~/`, `/abs`) fall through to the directory-listing # path so explicit navigation intent is preserved. - if ( - is_context - and path_part - and "/" not in path_part - and prefix_tag != "folder" - ): + if is_context and path_part and "/" not in path_part and prefix_tag != "folder": root = os.getcwd() ranked: list[tuple[tuple[int, int], str, str]] = [] for rel in _list_repo_files(root): @@ -3943,9 +3984,7 @@ def _(rid, params: dict) -> dict: voice_cfg = _load_cfg().get("voice", {}) start_continuous( - on_transcript=lambda t: _voice_emit( - "voice.transcript", {"text": t} - ), + on_transcript=lambda t: _voice_emit("voice.transcript", {"text": t}), on_status=lambda s: _voice_emit("voice.status", {"state": s}), on_silent_limit=lambda: _voice_emit( "voice.transcript", {"no_speech_limit": True} diff --git a/ui-tui/src/app/useSessionLifecycle.ts b/ui-tui/src/app/useSessionLifecycle.ts index acd10135e..baaf3fc3c 100644 --- a/ui-tui/src/app/useSessionLifecycle.ts +++ b/ui-tui/src/app/useSessionLifecycle.ts @@ -142,6 +142,10 @@ export function useSessionLifecycle(opts: UseSessionLifecycleOptions) { sys(`warning: ${info.credential_warning}`) } + if (info?.config_warning) { + sys(`warning: ${info.config_warning}`) + } + if (msg) { sys(msg) } diff --git a/ui-tui/src/gatewayTypes.ts b/ui-tui/src/gatewayTypes.ts index a38e06804..91fced32a 100644 --- a/ui-tui/src/gatewayTypes.ts +++ b/ui-tui/src/gatewayTypes.ts @@ -93,7 +93,7 @@ export interface SetupStatusResponse { // ── Session lifecycle ──────────────────────────────────────────────── export interface SessionCreateResponse { - info?: SessionInfo & { credential_warning?: string } + info?: SessionInfo & { config_warning?: string; credential_warning?: string } session_id: string }