diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 439d8cd66c..35f8f42290 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -7595,6 +7595,7 @@ def _denormalize_config_from_web(config: Dict[str, Any]) -> Dict[str, Any]: @app.put("/api/config") async def update_config(body: ConfigUpdate, profile: Optional[str] = None): def _run(): + approvals_mode_changed = False with _profile_scope(body.profile or profile): # The dashboard form is schema-driven (see CONFIG_SCHEMA). Any root # key absent from the schema — most visibly ``custom_providers``, but @@ -7605,7 +7606,23 @@ async def update_config(body: ConfigUpdate, profile: Optional[str] = None): with _CONFIG_MUTATION_LOCK: existing = read_raw_config() incoming = _denormalize_config_from_web(body.config) - save_config(_deep_merge(existing, incoming)) + merged = _deep_merge(existing, incoming) + # Compare normalized approvals.mode across the in-memory + # documents, not config blocks and not cache re-reads: the + # settings page PUTs the defaulted GET record while disk + # holds sparse YAML, so a block compare is always-unequal + # (every autosave would broadcast), and reloading after the + # save can serve the pre-save cache on an (mtime_ns, size) + # key collision. Only approvals.mode feeds session.info, so + # it is the honest trigger. + approvals_mode_changed = _approval_mode_of(merged) != _approval_mode_of(existing) + save_config(merged) + # REST saves bypass the config.set RPC (which re-emits itself), so + # refresh live sessions' cached approval/YOLO indicators after a mode + # change. Own-profile saves only: a profile-scoped save targets a + # different HERMES_HOME than this process's gateway sessions. + if approvals_mode_changed and not _is_other_profile(body.profile or profile): + _broadcast_gateway_session_info() return {"ok": True} try: @@ -7617,6 +7634,51 @@ async def update_config(body: ConfigUpdate, profile: Optional[str] = None): raise HTTPException(status_code=500, detail="Internal server error") +def _is_other_profile(profile: Optional[str]) -> bool: + """True when ``profile`` names a profile other than this process's own.""" + requested = (profile or "").strip() + if not requested or requested.lower() == "current": + return False + try: + target = _resolve_profile_dir(requested) + except HTTPException: + return True + return target.resolve() != get_process_hermes_home().resolve() + + +def _approval_mode_of(config: Dict[str, Any]) -> str: + """Normalize approvals.mode from an in-memory config document. + + Both sides of the broadcast comparison use in-memory documents (the raw + on-disk dict and the about-to-be-saved dict): re-reading through the + config cache after a save can serve the pre-save document when the + replacement file collides on the (mtime_ns, size) cache key, which would + suppress the broadcast exactly when the mode changed. Absent block or + key normalizes to the same default the approval gate uses. + """ + from tools.approval import _normalize_approval_mode + + approvals = config.get("approvals") + default_mode = (DEFAULT_CONFIG.get("approvals") or {}).get("mode", "manual") + mode = approvals.get("mode", default_mode) if isinstance(approvals, dict) else default_mode + return _normalize_approval_mode(mode) + + +def _broadcast_gateway_session_info() -> None: + """Broadcast session.info on the in-process gateway when it's loaded. + + ``sys.modules`` guard, not an import: gateway never imported means no + live sessions in this process to notify. + """ + server = sys.modules.get("tui_gateway.server") + if server is None: + return + try: + server.broadcast_session_info() + except Exception: + _log.exception("session.info broadcast after config save failed") + + def _catalog_provider_env_metadata() -> dict: """Map provider env vars → desktop card metadata, derived from the catalog. @@ -15160,10 +15222,15 @@ async def update_config_raw(body: RawConfigUpdate, profile: Optional[str] = None parsed = yaml.safe_load(body.yaml_text) if not isinstance(parsed, dict): raise HTTPException(status_code=400, detail="YAML must be a mapping") + approvals_mode_changed = False with _profile_scope(body.profile or profile): # Full-document replacement: the editor owns the whole file; do not # merge omitted sections back from disk (#62723). + approvals_mode_changed = _approval_mode_of(parsed) != _approval_mode_of(read_raw_config()) save_config(parsed, merge_existing=False) + # Same indicator refresh as the schema-driven save above. + if approvals_mode_changed and not _is_other_profile(body.profile or profile): + _broadcast_gateway_session_info() return {"ok": True} try: diff --git a/tests/hermes_cli/test_web_server_approvals_broadcast.py b/tests/hermes_cli/test_web_server_approvals_broadcast.py new file mode 100644 index 0000000000..cc4244a938 --- /dev/null +++ b/tests/hermes_cli/test_web_server_approvals_broadcast.py @@ -0,0 +1,226 @@ +"""Approvals config saves must re-emit session.info to live gateway sessions. + +Regression for the desktop "YOLO toggle does nothing / flips back off" bug: +the settings page saves ``approvals.mode`` through REST ``PUT /api/config`` +(and the raw editor through ``PUT /api/config/raw``), which wrote config.yaml +and emitted nothing. Enforcement follows the file immediately (the approval +gate re-reads config per command), but every live session's YOLO/approval +indicator repaints only on a ``session.info`` event, so the UI kept showing +stale bypass state, in the dangerous direction. The ``config.set`` RPC path +already re-emits after a mode flip; these tests pin the REST paths to the +same contract. +""" + +import types + +import pytest + + +@pytest.fixture +def client(_isolate_hermes_home): + try: + from starlette.testclient import TestClient + except ImportError: + pytest.skip("fastapi/starlette not installed") + from hermes_cli import web_server + + client = TestClient(web_server.app) + client.headers[web_server._SESSION_HEADER_NAME] = web_server._SESSION_TOKEN + return client + + +@pytest.fixture +def broadcast_calls(monkeypatch): + """Stub the in-memory gateway module seam and record broadcasts.""" + import sys + + calls = [] + # tests/conftest.py's session-reaper teardown walks tui_gateway.server + # attributes; the stub must carry an empty _sessions to survive it. + stub = types.SimpleNamespace( + broadcast_session_info=lambda: calls.append(True), + _sessions={}, + ) + monkeypatch.setitem(sys.modules, "tui_gateway.server", stub) + return calls + + +class TestApprovalsSaveBroadcast: + def test_get_shaped_record_roundtrip_does_not_broadcast(self, client, broadcast_calls): + """The settings page PUTs the defaulted GET record back verbatim on + every autosave. That must not broadcast: disk holds sparse YAML while + GET returns defaults, so a block-level compare is always-unequal (the + review-caught spam bug). Only an effective mode change may emit.""" + record = client.get("/api/config").json() + assert "approvals" in record + + first = client.put("/api/config", json={"config": record}) + assert first.status_code == 200 + second = client.put("/api/config", json={"config": record}) + assert second.status_code == 200 + assert not broadcast_calls, ( + "autosaving the unmodified GET record broadcast session.info; " + "every settings autosave would walk all live sessions" + ) + + flipped = {**record, "approvals": {**record["approvals"], "mode": "off"}} + resp = client.put("/api/config", json={"config": flipped}) + assert resp.status_code == 200 + assert len(broadcast_calls) == 1, ( + "an actual approvals.mode change in the GET-shaped record must " + "broadcast exactly once" + ) + + def test_approvals_mode_change_broadcasts(self, client, broadcast_calls): + resp = client.put("/api/config", json={"config": {"approvals": {"mode": "off"}}}) + assert resp.status_code == 200 + assert broadcast_calls, ( + "PUT /api/config changed approvals.mode but no session.info " + "broadcast reached the gateway, so live sessions keep painting " + "stale YOLO/approval state" + ) + + def test_non_approvals_change_does_not_broadcast(self, client, broadcast_calls): + resp = client.put("/api/config", json={"config": {"display": {"skin": "mono"}}}) + assert resp.status_code == 200 + assert not broadcast_calls, ( + "a save that never touched approvals must not spam session.info" + ) + + def test_approvals_noop_save_does_not_broadcast(self, client, broadcast_calls): + first = client.put("/api/config", json={"config": {"approvals": {"mode": "off"}}}) + assert first.status_code == 200 + broadcast_calls.clear() + + again = client.put("/api/config", json={"config": {"approvals": {"mode": "off"}}}) + assert again.status_code == 200 + assert not broadcast_calls, ( + "saving an identical approvals block is a no-op and must not " + "re-emit session.info" + ) + + def test_own_profile_named_default_broadcasts(self, client, broadcast_calls): + """Dashboard/desktop often send ?profile=default for this process's + own home. That is not an other-profile save and must still emit.""" + resp = client.put( + "/api/config?profile=default", + json={"config": {"approvals": {"mode": "off"}}}, + ) + assert resp.status_code == 200 + assert broadcast_calls, ( + "?profile=default is this process's own HERMES_HOME; skipping " + "the broadcast leaves live sessions painting stale YOLO state" + ) + + def test_other_profile_save_does_not_broadcast(self, client, broadcast_calls, monkeypatch, tmp_path): + from hermes_cli import web_server + + profile_dir = tmp_path / "profiles" / "other" + profile_dir.mkdir(parents=True) + monkeypatch.setattr(web_server, "_resolve_profile_dir", lambda name: profile_dir) + + resp = client.put( + "/api/config", + json={"config": {"approvals": {"mode": "off"}}, "profile": "other"}, + ) + assert resp.status_code == 200 + assert not broadcast_calls, ( + "a profile-scoped save targets a different HERMES_HOME than this " + "process's gateway sessions; broadcasting our own sessions' " + "unchanged state is wrong" + ) + + def test_gateway_not_imported_is_a_noop(self, client, monkeypatch): + import sys + + monkeypatch.delitem(sys.modules, "tui_gateway.server", raising=False) + resp = client.put("/api/config", json={"config": {"approvals": {"mode": "smart"}}}) + assert resp.status_code == 200 + assert "tui_gateway.server" not in sys.modules, ( + "the broadcast seam must not IMPORT the gateway; a process " + "without one has no sessions to notify" + ) + + def test_raw_save_deleting_approvals_block_broadcasts(self, client, broadcast_calls): + seed = client.put( + "/api/config/raw", + json={"yaml_text": "approvals:\n mode: 'off'\n"}, + ) + assert seed.status_code == 200 + broadcast_calls.clear() + + # Full-document replacement that drops the approvals block entirely: + # effective mode falls back to default (manual), so indicators must + # repaint. + resp = client.put( + "/api/config/raw", + json={"yaml_text": "display:\n skin: default\n"}, + ) + assert resp.status_code == 200 + assert broadcast_calls, ( + "deleting the approvals block changes the effective mode and " + "must broadcast" + ) + + def test_raw_save_approvals_change_broadcasts(self, client, broadcast_calls): + resp = client.put( + "/api/config/raw", + json={"yaml_text": "approvals:\n mode: 'off'\n"}, + ) + assert resp.status_code == 200 + assert broadcast_calls, ( + "PUT /api/config/raw changed approvals but no session.info " + "broadcast reached the gateway" + ) + + def test_raw_save_without_approvals_change_does_not_broadcast(self, client, broadcast_calls): + seed = client.put( + "/api/config/raw", + json={"yaml_text": "approvals:\n mode: manual\ndisplay:\n skin: default\n"}, + ) + assert seed.status_code == 200 + broadcast_calls.clear() + + resp = client.put( + "/api/config/raw", + json={"yaml_text": "approvals:\n mode: manual\ndisplay:\n skin: mono\n"}, + ) + assert resp.status_code == 200 + assert not broadcast_calls + + +class TestGatewayBroadcastHelper: + def test_broadcast_session_info_emits_for_live_sessions(self, _isolate_hermes_home, monkeypatch): + """tui_gateway.server.broadcast_session_info walks _sessions and emits.""" + from tui_gateway import server + + emitted = [] + monkeypatch.setattr( + server, "_emit_session_info_for_session", + lambda sid, sess: emitted.append(sid), + ) + monkeypatch.setattr( + server, "_sessions", + {"s1": {"agent": object()}, "s2": {"agent": object()}}, + ) + + server.broadcast_session_info() + + assert sorted(emitted) == ["s1", "s2"] + + def test_approvals_slash_mirror_broadcasts(self, _isolate_hermes_home, monkeypatch): + """/approvals through the slash worker persists config out of + band; the mirror must repaint live sessions and the bare read-only + form must not.""" + from tui_gateway import server + + calls = [] + monkeypatch.setattr(server, "broadcast_session_info", lambda: calls.append(True)) + + session = {"agent": None} + server._mirror_slash_side_effects("sid1", session, "/approvals off") + assert calls, "/approvals writes approvals.mode and must broadcast" + + calls.clear() + server._mirror_slash_side_effects("sid1", session, "/approvals") + assert not calls, "bare /approvals only reads the mode" diff --git a/tui_gateway/server.py b/tui_gateway/server.py index e7b09ab8c2..9493814bd0 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -5776,6 +5776,20 @@ def _emit_session_info_for_session(sid: str, session: dict) -> None: pass +def broadcast_session_info() -> None: + """Re-emit ``session.info`` to every live session. + + For approvals-config writers that bypass the ``config.set`` RPC (which + re-emits itself): the REST config saves and the ``/approvals`` slash + mirror. Only reaches sessions in THIS process; a spawned + ``tui_gateway.entry`` child gateway has its own ``_sessions``. + """ + with _sessions_lock: + sessions = list(_sessions.items()) + for sid, sess in sessions: + _emit_session_info_for_session(sid, sess) + + # Tool Args/Result text shipped to the TUI for the verbose trail line. The TUI # renders only a small persisted preview (ui-tui VERBOSE_TRAIL_MAX_CHARS), kept # all session and expanded by default — so shipping more than that is pure pipe @@ -13939,6 +13953,10 @@ def _mirror_slash_side_effects(sid: str, session: dict, command: str) -> str: if name == "model" and arg and agent: result = _apply_model_switch(sid, session, arg) return result.get("warning", "") + elif name == "approvals" and arg: + # The slash worker already persisted the new approvals.mode; the + # bare (read-only) form has no arg and needs no repaint. + broadcast_session_info() elif name == "personality" and arg and agent: pname, new_prompt = _validate_personality(arg, _load_cfg()) # Persist through the single owner so this surface can never