diff --git a/hermes_cli/profile_cmd.py b/hermes_cli/profile_cmd.py index caf12315ea..228b389647 100644 --- a/hermes_cli/profile_cmd.py +++ b/hermes_cli/profile_cmd.py @@ -245,11 +245,17 @@ def _profile_create(args): _print_channel_clone_notice(name, source_label, clone_channels, "--clone-all" if clone_all else "--clone") # Auto-clone Honcho config for the new profile (only with clone operations) try: - from plugins.memory.honcho.cli import clone_honcho_for_profile - if clone_honcho_for_profile(name): - print(f"Honcho config cloned (peer: {name})") + from plugins.memory.honcho.cli import ConfigWriteRefused, clone_honcho_for_profile except Exception: - pass # Honcho plugin not installed or not configured + clone_honcho_for_profile = None # Honcho plugin not installed + if clone_honcho_for_profile is not None: + try: + if clone_honcho_for_profile(name): + print(f"Honcho config cloned (peer: {name})") + except ConfigWriteRefused as e: + print(f"Honcho config not cloned: {e}") + except Exception: + pass # Honcho not configured else: # Fresh profiles only: clones already carry the source's (user-curated) skills. result = seed_profile_skills(profile_dir) diff --git a/hermes_cli/web_routers/memory_providers.py b/hermes_cli/web_routers/memory_providers.py index 11485d2292..83c71ca573 100644 --- a/hermes_cli/web_routers/memory_providers.py +++ b/hermes_cli/web_routers/memory_providers.py @@ -178,7 +178,7 @@ def _write_provider_flat(provider: ProviderConfigSchema, values: Dict[str, str]) def _write_provider_honcho(provider: ProviderConfigSchema, values: Dict[str, str]) -> None: """Persist submitted fields to Honcho's real config for the active host (partial saves touch only submitted keys; blank text clears a key — see ``_apply_field_values``).""" - from plugins.memory.honcho.oauth import ACCESS_TOKEN_PREFIX, _config_refresh_lock + from plugins.memory.honcho.oauth import ACCESS_TOKEN_PREFIX, _config_refresh_lock, _read_config_strict, _refresh_lock resolve_active_host, resolve_config_path, host_block_of = _honcho_resolvers() host = resolve_active_host() @@ -186,8 +186,9 @@ def _write_provider_honcho(provider: ProviderConfigSchema, values: Dict[str, str path = resolve_config_path() # OAuth rotation is single-use; an unlocked RMW here can revoke the grant. - with _config_refresh_lock(path): - cfg = _read_json_dict(path, "Honcho config") + with _refresh_lock, _config_refresh_lock(path): + # Strict: a file that exists but does not parse must not be replaced by this host's block alone. + cfg = _read_config_strict(path) hosts = cfg.get("hosts") cfg["hosts"] = hosts = hosts if isinstance(hosts, dict) else {} # Update the block reads resolve (legacy dot-form included), never shadow it. diff --git a/plugins/memory/honcho/cli.py b/plugins/memory/honcho/cli.py index 36a5c94703..e4e7e475ce 100644 --- a/plugins/memory/honcho/cli.py +++ b/plugins/memory/honcho/cli.py @@ -135,10 +135,11 @@ def _write_config(cfg: dict, path: Path | None = None) -> None: """Persist ``cfg`` under the token refresh's cross-process lock. The object _read_config() returned has only its edits applied onto a fresh read of disk; a plain dict is written whole. A read that resolved to a seed file (~/.honcho or a profile) is written whole only while ``path`` does not exist.""" - from plugins.memory.honcho.oauth import _config_refresh_lock, _read_config_strict + from plugins.memory.honcho.oauth import _config_refresh_lock, _read_config_strict, _refresh_lock from utils import atomic_json_write path = path or _local_config_path() - with _config_refresh_lock(path): + # The file lock is best-effort; _refresh_lock is what keeps an in-process refresh thread out. + with _refresh_lock, _config_refresh_lock(path): _refuse_unparseable(path) out = cfg if getattr(cfg, "path", None) == path: diff --git a/tests/hermes_cli/test_web_memory_providers_honcho_write.py b/tests/hermes_cli/test_web_memory_providers_honcho_write.py new file mode 100644 index 0000000000..dd04ec2992 --- /dev/null +++ b/tests/hermes_cli/test_web_memory_providers_honcho_write.py @@ -0,0 +1,32 @@ +"""The dashboard's Honcho save path shares the plugin's write invariants: it refuses to rewrite a +honcho.json that exists but does not parse, and it merges into a parseable one.""" + +import json + +import pytest + +from hermes_cli.web_routers import memory_providers as mp +from plugins.memory.honcho.config_schema import CONFIG_SCHEMA + + +def _point_at(monkeypatch, path): + from plugins.memory.honcho.client import _host_block + monkeypatch.setattr(mp, "_honcho_resolvers", lambda: (lambda: "hermes", lambda: path, _host_block)) + + +@pytest.mark.parametrize("corrupt", [True, False], ids=["unparseable-file-is-left-alone", "parseable-file-is-merged"]) +def test_web_save_never_replaces_an_unparseable_honcho_json(tmp_path, monkeypatch, corrupt): + path = tmp_path / "honcho.json" + before = "{not json" if corrupt else json.dumps({"hosts": {"other": {"apiKey": "keep-me"}}}) + path.write_text(before, encoding="utf-8") + _point_at(monkeypatch, path) + + if corrupt: + with pytest.raises(ValueError): + mp._write_provider_honcho(CONFIG_SCHEMA, {"recallMode": "tools"}) + assert path.read_text(encoding="utf-8") == before + return + mp._write_provider_honcho(CONFIG_SCHEMA, {"recallMode": "tools"}) + data = json.loads(path.read_text(encoding="utf-8")) + assert data["hosts"]["other"]["apiKey"] == "keep-me" + assert data["hosts"]["hermes"]["recallMode"] == "tools"