mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
feat(tui): warn on bare null sections in config.yaml
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.
This commit is contained in:
parent
fd9b692d33
commit
bfa60234c8
4 changed files with 106 additions and 38 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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}
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue