mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-13 03:52:00 +00:00
fix(tui): close slash parity gaps with CLI (#20339)
* fix(tui): close slash parity gaps with CLI Route unsupported /skills subcommands through slash.exec, support /new <name> titles, and handle /redraw natively so TUI behavior matches classic CLI. Also filter gateway-only commands out of the TUI catalog while keeping /status discoverable. * fix(tui): run remaining CLI parity paths natively Forward chat launch flags into the TUI runtime and handle live-session status and skill reloads in the gateway process so TUI state no longer depends on the slash worker's stale CLI instance. * fix(tui): block stale snapshot restores Prevent snapshot restore from running through the isolated slash worker because it mutates disk state without refreshing the live TUI agent. * chore: uptick * fix(tui): guard async session title updates Handle failures from the fire-and-forget session.title RPC so title-setting errors do not surface as unhandled promise rejections while preserving session-scoped messaging.
This commit is contained in:
brooklyn!
GitHub
parent
acca3ec3af
commit
794f48766c
14 changed files with 1266 additions and 284 deletions
|
|
@ -70,9 +70,7 @@ def test_dispatch_rejects_non_object_request():
|
|||
|
||||
|
||||
def test_dispatch_rejects_non_object_params():
|
||||
resp = server.dispatch(
|
||||
{"id": "1", "method": "session.create", "params": []}
|
||||
)
|
||||
resp = server.dispatch({"id": "1", "method": "session.create", "params": []})
|
||||
|
||||
assert resp == {
|
||||
"jsonrpc": "2.0",
|
||||
|
|
@ -133,12 +131,16 @@ def test_voice_toggle_handles_non_dict_voice_cfg(monkeypatch):
|
|||
monkeypatch.setattr(server, "_load_cfg", lambda b=bad: {"voice": b})
|
||||
|
||||
status_resp = server.dispatch(
|
||||
{"id": "voice-status", "method": "voice.toggle", "params": {"action": "status"}}
|
||||
{
|
||||
"id": "voice-status",
|
||||
"method": "voice.toggle",
|
||||
"params": {"action": "status"},
|
||||
}
|
||||
)
|
||||
|
||||
assert status_resp["result"]["record_key"] == "ctrl+b", (
|
||||
f"voice.record_key fell back to default for voice={bad!r}"
|
||||
)
|
||||
assert (
|
||||
status_resp["result"]["record_key"] == "ctrl+b"
|
||||
), f"voice.record_key fell back to default for voice={bad!r}"
|
||||
|
||||
# Round-4 follow-up: the YAML root itself may be a non-dict. A
|
||||
# hand-edit that collapses config.yaml to a scalar / list would
|
||||
|
|
@ -148,12 +150,16 @@ def test_voice_toggle_handles_non_dict_voice_cfg(monkeypatch):
|
|||
monkeypatch.setattr(server, "_load_cfg", lambda r=bad_root: r)
|
||||
|
||||
status_resp = server.dispatch(
|
||||
{"id": "voice-status-root", "method": "voice.toggle", "params": {"action": "status"}}
|
||||
{
|
||||
"id": "voice-status-root",
|
||||
"method": "voice.toggle",
|
||||
"params": {"action": "status"},
|
||||
}
|
||||
)
|
||||
|
||||
assert status_resp["result"]["record_key"] == "ctrl+b", (
|
||||
f"voice.record_key fell back to default for root={bad_root!r}"
|
||||
)
|
||||
assert (
|
||||
status_resp["result"]["record_key"] == "ctrl+b"
|
||||
), f"voice.record_key fell back to default for root={bad_root!r}"
|
||||
|
||||
|
||||
def test_voice_record_start_handles_non_dict_voice_cfg(monkeypatch):
|
||||
|
|
@ -174,7 +180,9 @@ def test_voice_record_start_handles_non_dict_voice_cfg(monkeypatch):
|
|||
monkeypatch.setitem(
|
||||
sys.modules,
|
||||
"hermes_cli.voice",
|
||||
types.SimpleNamespace(start_continuous=fake_start_continuous, stop_continuous=lambda: None),
|
||||
types.SimpleNamespace(
|
||||
start_continuous=fake_start_continuous, stop_continuous=lambda: None
|
||||
),
|
||||
)
|
||||
monkeypatch.setenv("HERMES_VOICE", "1")
|
||||
|
||||
|
|
@ -183,10 +191,16 @@ def test_voice_record_start_handles_non_dict_voice_cfg(monkeypatch):
|
|||
monkeypatch.setattr(server, "_load_cfg", lambda b=bad: {"voice": b})
|
||||
|
||||
resp = server.dispatch(
|
||||
{"id": "voice-record", "method": "voice.record", "params": {"action": "start"}}
|
||||
{
|
||||
"id": "voice-record",
|
||||
"method": "voice.record",
|
||||
"params": {"action": "start"},
|
||||
}
|
||||
)
|
||||
|
||||
assert "result" in resp, f"voice.record raised for voice={bad!r}: {resp.get('error')}"
|
||||
assert (
|
||||
"result" in resp
|
||||
), f"voice.record raised for voice={bad!r}: {resp.get('error')}"
|
||||
assert resp["result"]["status"] == "recording"
|
||||
assert captured["silence_threshold"] == 200
|
||||
assert captured["silence_duration"] == 3.0
|
||||
|
|
@ -204,16 +218,20 @@ def test_voice_record_start_handles_non_dict_voice_cfg(monkeypatch):
|
|||
monkeypatch.setattr(server, "_load_cfg", lambda c=bad_bool_cfg: {"voice": c})
|
||||
|
||||
resp = server.dispatch(
|
||||
{"id": "voice-record-bool", "method": "voice.record", "params": {"action": "start"}}
|
||||
{
|
||||
"id": "voice-record-bool",
|
||||
"method": "voice.record",
|
||||
"params": {"action": "start"},
|
||||
}
|
||||
)
|
||||
|
||||
assert "result" in resp, f"voice.record raised for bool cfg={bad_bool_cfg!r}"
|
||||
assert captured["silence_threshold"] == 200, (
|
||||
f"bool silence_threshold leaked through for {bad_bool_cfg!r}"
|
||||
)
|
||||
assert captured["silence_duration"] == 3.0, (
|
||||
f"bool silence_duration leaked through for {bad_bool_cfg!r}"
|
||||
)
|
||||
assert (
|
||||
captured["silence_threshold"] == 200
|
||||
), f"bool silence_threshold leaked through for {bad_bool_cfg!r}"
|
||||
assert (
|
||||
captured["silence_duration"] == 3.0
|
||||
), f"bool silence_duration leaked through for {bad_bool_cfg!r}"
|
||||
|
||||
|
||||
def test_voice_toggle_tts_branch_also_carries_record_key(monkeypatch):
|
||||
|
|
@ -281,7 +299,9 @@ def test_load_enabled_toolsets_accepts_plugin_env_after_discovery(monkeypatch):
|
|||
monkeypatch.setitem(
|
||||
sys.modules,
|
||||
"hermes_cli.plugins",
|
||||
types.SimpleNamespace(discover_plugins=lambda: discovered.update({"ready": True})),
|
||||
types.SimpleNamespace(
|
||||
discover_plugins=lambda: discovered.update({"ready": True})
|
||||
),
|
||||
)
|
||||
|
||||
assert server._load_enabled_toolsets() == ["plugin_demo"]
|
||||
|
|
@ -302,7 +322,9 @@ def test_load_enabled_toolsets_rejects_disabled_mcp_env(monkeypatch, capsys):
|
|||
"read_raw_config",
|
||||
lambda: {"mcp_servers": {"mcp-off": {"enabled": False}}},
|
||||
)
|
||||
monkeypatch.setattr(config_mod, "load_config", lambda: {"platform_toolsets": {"cli": ["memory"]}})
|
||||
monkeypatch.setattr(
|
||||
config_mod, "load_config", lambda: {"platform_toolsets": {"cli": ["memory"]}}
|
||||
)
|
||||
|
||||
# Sorted: ["kanban", "memory"]. `kanban` is auto-recovered by
|
||||
# _get_platform_tools because it's a non-configurable platform toolset
|
||||
|
|
@ -324,7 +346,9 @@ def test_load_enabled_toolsets_falls_back_when_tui_env_invalid(monkeypatch, caps
|
|||
|
||||
import hermes_cli.config as config_mod
|
||||
|
||||
monkeypatch.setattr(config_mod, "load_config", lambda: {"platform_toolsets": {"cli": ["memory"]}})
|
||||
monkeypatch.setattr(
|
||||
config_mod, "load_config", lambda: {"platform_toolsets": {"cli": ["memory"]}}
|
||||
)
|
||||
|
||||
assert server._load_enabled_toolsets() == ["kanban", "memory"]
|
||||
assert "using configured CLI toolsets" in capsys.readouterr().err
|
||||
|
|
@ -340,7 +364,9 @@ def test_load_enabled_toolsets_warns_when_config_fallback_fails(monkeypatch, cap
|
|||
|
||||
import hermes_cli.config as config_mod
|
||||
|
||||
monkeypatch.setattr(config_mod, "load_config", lambda: (_ for _ in ()).throw(RuntimeError("boom")))
|
||||
monkeypatch.setattr(
|
||||
config_mod, "load_config", lambda: (_ for _ in ()).throw(RuntimeError("boom"))
|
||||
)
|
||||
|
||||
assert server._load_enabled_toolsets() is None
|
||||
assert "could not be loaded" in capsys.readouterr().err
|
||||
|
|
@ -351,7 +377,9 @@ def test_load_enabled_toolsets_honors_builtin_env_if_config_fails(monkeypatch):
|
|||
|
||||
import hermes_cli.config as config_mod
|
||||
|
||||
monkeypatch.setattr(config_mod, "load_config", lambda: (_ for _ in ()).throw(RuntimeError("boom")))
|
||||
monkeypatch.setattr(
|
||||
config_mod, "load_config", lambda: (_ for _ in ()).throw(RuntimeError("boom"))
|
||||
)
|
||||
|
||||
assert server._load_enabled_toolsets() == ["web"]
|
||||
|
||||
|
|
@ -362,7 +390,9 @@ def test_load_enabled_toolsets_all_env_means_all(monkeypatch):
|
|||
assert server._load_enabled_toolsets() is None
|
||||
|
||||
|
||||
def test_load_enabled_toolsets_all_env_warns_about_ignored_extra_entries(monkeypatch, capsys):
|
||||
def test_load_enabled_toolsets_all_env_warns_about_ignored_extra_entries(
|
||||
monkeypatch, capsys
|
||||
):
|
||||
monkeypatch.setenv("HERMES_TUI_TOOLSETS", "all,nope")
|
||||
|
||||
assert server._load_enabled_toolsets() is None
|
||||
|
|
@ -1801,9 +1831,7 @@ def test_session_compress_uses_compress_helper(monkeypatch):
|
|||
emit.assert_any_call("session.info", "sid", {"model": "x"})
|
||||
# Final status.update clears the pinned "compressing" indicator so the
|
||||
# status bar can revert to the neutral state when compaction finishes.
|
||||
emit.assert_any_call(
|
||||
"status.update", "sid", {"kind": "status", "text": "ready"}
|
||||
)
|
||||
emit.assert_any_call("status.update", "sid", {"kind": "status", "text": "ready"})
|
||||
|
||||
|
||||
def test_session_compress_syncs_session_key_after_rotation(monkeypatch):
|
||||
|
|
@ -2050,6 +2078,120 @@ def test_commands_catalog_includes_tui_mouse_command():
|
|||
assert "/mouse" in tui_pairs
|
||||
|
||||
|
||||
def test_commands_catalog_filters_gateway_only_commands_and_keeps_status_visible():
|
||||
resp = server.handle_request(
|
||||
{"id": "1", "method": "commands.catalog", "params": {}}
|
||||
)
|
||||
|
||||
pairs = dict(resp["result"]["pairs"])
|
||||
canon = resp["result"]["canon"]
|
||||
|
||||
assert "/status" in pairs
|
||||
assert canon["/status"] == "/status"
|
||||
|
||||
assert "/topic" not in pairs
|
||||
assert "/approve" not in pairs
|
||||
assert "/deny" not in pairs
|
||||
assert "/sethome" not in pairs
|
||||
|
||||
assert "/topic" not in canon
|
||||
assert "/approve" not in canon
|
||||
assert "/deny" not in canon
|
||||
assert "/set-home" not in canon
|
||||
|
||||
|
||||
def test_session_status_reads_live_gateway_agent(monkeypatch):
|
||||
agent = types.SimpleNamespace(
|
||||
model="live-model",
|
||||
provider="live-provider",
|
||||
session_total_tokens=1234,
|
||||
)
|
||||
server._sessions["sid"] = _session(agent=agent, running=True)
|
||||
|
||||
class _DB:
|
||||
def get_session(self, key):
|
||||
assert key == "session-key"
|
||||
return {
|
||||
"title": "Live TUI",
|
||||
"started_at": 1_700_000_000,
|
||||
"updated_at": 1_700_000_060,
|
||||
}
|
||||
|
||||
monkeypatch.setattr(server, "_get_db", lambda: _DB())
|
||||
try:
|
||||
resp = server.handle_request(
|
||||
{"id": "1", "method": "session.status", "params": {"session_id": "sid"}}
|
||||
)
|
||||
finally:
|
||||
server._sessions.pop("sid", None)
|
||||
|
||||
out = resp["result"]["output"]
|
||||
assert "Hermes TUI Status" in out
|
||||
assert "Session ID: session-key" in out
|
||||
assert "Title: Live TUI" in out
|
||||
assert "Model: live-model (live-provider)" in out
|
||||
assert "Tokens: 1,234" in out
|
||||
assert "Agent Running: Yes" in out
|
||||
|
||||
|
||||
def test_skills_reload_runs_in_gateway_process(monkeypatch):
|
||||
import agent.skill_commands as skill_commands
|
||||
|
||||
called = {}
|
||||
monkeypatch.setattr(
|
||||
skill_commands,
|
||||
"reload_skills",
|
||||
lambda: called.setdefault(
|
||||
"result",
|
||||
{
|
||||
"added": [{"name": "new-skill", "description": "demo"}],
|
||||
"removed": [],
|
||||
"total": 42,
|
||||
},
|
||||
),
|
||||
)
|
||||
|
||||
resp = server.handle_request({"id": "1", "method": "skills.reload", "params": {}})
|
||||
|
||||
assert called["result"]["total"] == 42
|
||||
assert "new-skill" in resp["result"]["output"]
|
||||
assert "42 skill(s) available" in resp["result"]["output"]
|
||||
|
||||
|
||||
def test_snapshot_restore_is_blocked_from_tui_worker():
|
||||
server._sessions["sid"] = _session()
|
||||
try:
|
||||
worker_resp = server.handle_request(
|
||||
{
|
||||
"id": "1",
|
||||
"method": "slash.exec",
|
||||
"params": {"command": "snapshot restore latest", "session_id": "sid"},
|
||||
}
|
||||
)
|
||||
dispatch_resp = server.handle_request(
|
||||
{
|
||||
"id": "2",
|
||||
"method": "command.dispatch",
|
||||
"params": {
|
||||
"arg": "restore latest",
|
||||
"name": "snapshot",
|
||||
"session_id": "sid",
|
||||
},
|
||||
}
|
||||
)
|
||||
finally:
|
||||
server._sessions.pop("sid", None)
|
||||
|
||||
assert worker_resp["error"]["code"] == 4018
|
||||
assert (
|
||||
"snapshot restore mutates live config/state" in worker_resp["error"]["message"]
|
||||
)
|
||||
assert dispatch_resp["result"]["type"] == "exec"
|
||||
assert (
|
||||
"/snapshot restore is blocked in the TUI" in dispatch_resp["result"]["output"]
|
||||
)
|
||||
|
||||
|
||||
def test_command_dispatch_exec_nonzero_surfaces_error(monkeypatch):
|
||||
monkeypatch.setattr(
|
||||
server,
|
||||
|
|
@ -4161,9 +4303,7 @@ def test_reload_env_rpc_calls_hermes_cli_reload_env(monkeypatch):
|
|||
|
||||
fake = types.SimpleNamespace(reload_env=_fake_reload)
|
||||
with patch.dict(sys.modules, {"hermes_cli.config": fake}):
|
||||
resp = server.handle_request(
|
||||
{"id": "1", "method": "reload.env", "params": {}}
|
||||
)
|
||||
resp = server.handle_request({"id": "1", "method": "reload.env", "params": {}})
|
||||
|
||||
assert resp["result"] == {"updated": 7}
|
||||
assert calls["n"] == 1
|
||||
|
|
@ -4175,9 +4315,7 @@ def test_reload_env_rpc_surfaces_errors(monkeypatch):
|
|||
|
||||
fake = types.SimpleNamespace(reload_env=_broken)
|
||||
with patch.dict(sys.modules, {"hermes_cli.config": fake}):
|
||||
resp = server.handle_request(
|
||||
{"id": "1", "method": "reload.env", "params": {}}
|
||||
)
|
||||
resp = server.handle_request({"id": "1", "method": "reload.env", "params": {}})
|
||||
|
||||
assert "error" in resp
|
||||
assert "env path locked" in resp["error"]["message"]
|
||||
|
|
@ -4188,7 +4326,9 @@ def test_reload_env_rpc_surfaces_errors(monkeypatch):
|
|||
|
||||
def _setup_make_agent_mocks(monkeypatch, cfg):
|
||||
monkeypatch.setattr(server, "_load_cfg", lambda: cfg)
|
||||
monkeypatch.setattr(server, "_resolve_startup_runtime", lambda: ("test-model", None))
|
||||
monkeypatch.setattr(
|
||||
server, "_resolve_startup_runtime", lambda: ("test-model", None)
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.runtime_provider.resolve_runtime_provider",
|
||||
lambda requested=None, target_model=None: {
|
||||
|
|
@ -4219,7 +4359,9 @@ def test_make_agent_reads_nested_max_turns(monkeypatch):
|
|||
|
||||
|
||||
def test_make_agent_nested_max_turns_takes_priority(monkeypatch):
|
||||
_setup_make_agent_mocks(monkeypatch, {"agent": {"max_turns": 500}, "max_turns": 100})
|
||||
_setup_make_agent_mocks(
|
||||
monkeypatch, {"agent": {"max_turns": 500}, "max_turns": 100}
|
||||
)
|
||||
|
||||
with patch("run_agent.AIAgent") as mock_agent:
|
||||
server._make_agent("sid1", "key1")
|
||||
|
|
@ -4309,6 +4451,8 @@ def test_config_show_displays_nested_max_turns(monkeypatch):
|
|||
|
||||
resp = server.handle_request({"id": "1", "method": "config.show", "params": {}})
|
||||
sections = resp["result"]["sections"]
|
||||
agent_rows = next(section["rows"] for section in sections if section["title"] == "Agent")
|
||||
agent_rows = next(
|
||||
section["rows"] for section in sections if section["title"] == "Agent"
|
||||
)
|
||||
|
||||
assert ["Max Turns", "120"] in agent_rows
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue