Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions plugins/memory/honcho/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,8 @@ def _resolve_api_key(cfg: dict) -> str:
config shapes, e.g. ``localhost:8000``) still pass — the Honcho SDK
will reject them itself with a clearer error than ours.
"""
host_key = _host_block(cfg, _host_key()).get("apiKey")
key = host_key or cfg.get("apiKey", "") or os.environ.get("HONCHO_API_KEY", "")
from plugins.memory.honcho.client import resolve_api_key_from_raw
key = resolve_api_key_from_raw(cfg, _host_key()) or ""
if not key:
base_url = cfg.get("baseUrl") or cfg.get("base_url") or os.environ.get("HONCHO_BASE_URL", "")
base_url = (base_url or "").strip()
Expand Down
132 changes: 100 additions & 32 deletions plugins/memory/honcho/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,22 +76,110 @@ def resolve_global_config_path() -> Path:
return Path.home() / ".honcho" / "config.json"


def _resolve_sticky_profile_name() -> str | None:
"""Named profile when HERMES_HOME is the default root (gateway without env)."""
home = get_hermes_home().resolve()
default = _get_default_hermes_home().resolve()
if home != default:
return None
try:
from hermes_cli.profiles import get_active_profile
name = get_active_profile()
if name and name != "default":
return name
except Exception:
pass
return None


def _profile_honcho_config_path() -> Path | None:
"""Per-profile honcho.json when sticky profile is active but HERMES_HOME is default."""
profile = _resolve_sticky_profile_name()
if not profile:
return None
path = _get_default_hermes_home() / "profiles" / profile / "honcho.json"
return path if path.exists() else None


def _read_honcho_json(path: Path) -> dict | None:
if not path.exists():
return None
try:
data = json.loads(path.read_text(encoding="utf-8"))
return data if isinstance(data, dict) else None
except (json.JSONDecodeError, OSError):
return None


def _merge_honcho_config(base: dict, overlay: dict) -> dict:
"""Deep-merge overlay onto base (profile overrides default host blocks)."""
merged = dict(base)
for key, val in overlay.items():
if key == "hosts" and isinstance(val, dict):
hosts = dict(merged.get("hosts") or {})
for hname, hblock in val.items():
if isinstance(hblock, dict) and isinstance(hosts.get(hname), dict):
hosts[hname] = {**hosts[hname], **hblock}
else:
hosts[hname] = hblock
merged["hosts"] = hosts
else:
merged[key] = val
return merged


def load_honcho_config_raw(config_path: Path | None = None) -> tuple[dict | None, Path]:
"""Load honcho config, merging profile overrides with the default profile file."""
path = config_path or resolve_config_path()
raw = _read_honcho_json(path)
profile_path = _profile_honcho_config_path()
default_path = _get_default_hermes_home() / "honcho.json"
if profile_path and path.resolve() == profile_path.resolve():
base = _read_honcho_json(default_path)
if base:
raw = _merge_honcho_config(base, raw or {})
return raw, path


def resolve_api_key_from_raw(raw: dict, host: str) -> str | None:
"""Resolve apiKey: host block → default hermes host → root → env."""
hosts = raw.get("hosts") or {}
key = _host_block(raw, host).get("apiKey")
if key:
return key
if host != HOST:
key = hosts.get(HOST, {}).get("apiKey")
if key:
return key
return raw.get("apiKey") or os.environ.get("HONCHO_API_KEY")


def resolve_config_path() -> Path:
"""Return the active Honcho config path.

Resolution order:
1. $HERMES_HOME/honcho.json (profile-local, if it exists)
2. ~/.hermes/honcho.json (default profile — shared host blocks live here)
3. ~/.honcho/config.json (global, cross-app interop)
2. ~/.hermes/profiles/<name>/honcho.json (sticky active profile)
3. ~/.hermes/honcho.json (default profile — shared host blocks live here)
4. ~/.honcho/config.json (global, cross-app interop)

Returns the global path if none exist (for first-time setup writes).
"""
local_path = get_hermes_home() / "honcho.json"
default_path = _get_default_hermes_home() / "honcho.json"
profile_path = _profile_honcho_config_path()
# Sticky profile honcho.json overrides the default-root file when both exist
# but HERMES_HOME was not propagated to the subprocess (issue #36098).
if profile_path is not None and local_path.resolve() == default_path.resolve():
return profile_path

if local_path.exists():
return local_path

if profile_path is not None:
return profile_path

# Default profile's config — host blocks accumulate here via setup/clone
default_path = _get_default_hermes_home() / "honcho.json"
if default_path != local_path and default_path.exists():
return default_path

Expand Down Expand Up @@ -207,11 +295,9 @@ def _parse_dialectic_depth_levels(host_val, root_val, depth: int) -> list[str] |

# Default HTTP timeout (seconds) applied when no explicit timeout is
# configured via HonchoClientConfig.timeout, honcho.timeout / requestTimeout,
# or HONCHO_TIMEOUT. Honcho calls happen on the post-response path of
# run_conversation; without a cap the agent can block indefinitely when
# the Honcho backend is unreachable, preventing the gateway from
# delivering the already-generated response.
_DEFAULT_HTTP_TIMEOUT = 30.0
# or HONCHO_TIMEOUT. Dialectic queries at reasoning_level≥medium often
# exceed 30s on self-hosted backends; 60s is a safer default cap.
_DEFAULT_HTTP_TIMEOUT = 60.0


def _resolve_optional_float(*values: Any) -> float | None:
Expand Down Expand Up @@ -413,16 +499,11 @@ def from_global_config(
"""
resolved_host = host or resolve_active_host()
path = config_path or resolve_config_path()
if not path.exists():
raw, _ = load_honcho_config_raw(path)
if raw is None:
logger.debug("No global Honcho config at %s, falling back to env", path)
return cls.from_env(host=resolved_host)

try:
raw = json.loads(path.read_text(encoding="utf-8"))
except (json.JSONDecodeError, OSError) as e:
logger.warning("Failed to read %s: %s, falling back to env", path, e)
return cls.from_env(host=resolved_host)

host_block = _host_block(raw, resolved_host)
# A hosts.hermes block or explicit enabled flag means the user
# intentionally configured Honcho for this host.
Expand All @@ -439,11 +520,7 @@ def from_global_config(
or raw.get("aiPeer")
or resolved_host
)
api_key = (
host_block.get("apiKey")
or raw.get("apiKey")
or os.environ.get("HONCHO_API_KEY")
)
api_key = resolve_api_key_from_raw(raw, resolved_host)

environment = (
host_block.get("environment")
Expand Down Expand Up @@ -818,24 +895,15 @@ def get_honcho_client(config: HonchoClientConfig | None = None) -> Honcho:
logger.info("Initializing Honcho client (host: %s, workspace: %s)", config.host, config.workspace_id)

# Local Honcho instances don't require an API key, but the SDK
# expects a non-empty string. Use a placeholder for local URLs.
# For local: only use config.api_key if the host block explicitly
# sets apiKey (meaning the user wants local auth). Otherwise skip
# the stored key -- it's likely a cloud key that would break local.
# expects a non-empty string. Use the resolved key when present;
# otherwise fall back to the "local" placeholder for open local stacks.
_is_local = resolved_base_url and (
"localhost" in resolved_base_url
or "127.0.0.1" in resolved_base_url
or "::1" in resolved_base_url
)
if _is_local:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This removes the explicit local-auth opt-in. Current main intentionally substitutes "local" unless this host block has apiKey, so a stored cloud/default key cannot break an unauthenticated loopback stack (827ce602d; current client.py:879-887). Please preserve that boundary and add an explicit shared-local-JWT mechanism if inheritance is required.

# Check if the host block has its own apiKey (explicit local auth).
# Auth-skipping is loopback-only: a stored key is likely a cloud key
# that would break a no-auth local server, so we substitute the SDK's
# required-non-empty placeholder unless the host block opts in.
_raw = config.raw or {}
_host_block = (_raw.get("hosts") or {}).get(config.host, {})
_host_has_key = bool(_host_block.get("apiKey"))
effective_api_key = config.api_key if _host_has_key else "local"
effective_api_key = (config.api_key or "").strip() or "local"
else:
effective_api_key = config.api_key

Expand Down
2 changes: 1 addition & 1 deletion plugins/memory/honcho/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,7 @@ def dialectic_query(
return result
except Exception as e:
logger.warning("Honcho dialectic query failed: %s", e)
return ""
return f"[honcho_error: {type(e).__name__}]"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This non-empty marker is treated as a successful dialectic result by automatic prefetch: it is stored, resets _dialectic_empty_streak, and is appended into injected context (plugins/memory/honcho/__init__.py:722-729, 854-861, 767-768). Classify failures separately or prevent markers from entering those paths.


def prefetch_context(self, session_key: str, user_message: str | None = None) -> None:
"""
Expand Down
96 changes: 93 additions & 3 deletions tests/honcho_plugin/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@
profile_host_key,
reset_honcho_client,
resolve_active_host,
resolve_api_key_from_raw,
resolve_config_path,
resolve_global_config_path,
_DEFAULT_HTTP_TIMEOUT,
)


Expand Down Expand Up @@ -655,9 +657,7 @@ def test_hermes_config_timeout_override_used_when_config_timeout_missing(self):
not importlib.util.find_spec("honcho"),
reason="honcho SDK not installed"
)
def test_defaults_to_30s_when_no_timeout_configured(self):
from plugins.memory.honcho.client import _DEFAULT_HTTP_TIMEOUT

def test_defaults_to_60s_when_no_timeout_configured(self):
fake_honcho = MagicMock(name="Honcho")
cfg = HonchoClientConfig(
api_key="test-key",
Expand Down Expand Up @@ -913,6 +913,96 @@ def test_depth_levels_invalid_values_default_to_low(self, tmp_path):
assert config.dialectic_depth_levels == ["low", "high"]


class TestResolveApiKeyFromRaw:
def test_profile_host_inherits_default_hermes_api_key(self):
raw = {
"apiKey": "root-key",
"hosts": {
"hermes": {"apiKey": "default-host-key"},
"hermes.coder": {"aiPeer": "hermes.coder"},
},
}
assert resolve_api_key_from_raw(raw, "hermes.coder") == "default-host-key"

def test_profile_host_block_wins_over_default(self):
raw = {
"hosts": {
"hermes": {"apiKey": "default-host-key"},
"hermes.coder": {"apiKey": "profile-key"},
},
}
assert resolve_api_key_from_raw(raw, "hermes.coder") == "profile-key"

def test_falls_back_to_root_then_env(self, monkeypatch):
monkeypatch.delenv("HONCHO_API_KEY", raising=False)
raw = {"apiKey": "root-key", "hosts": {"hermes.coder": {}}}
assert resolve_api_key_from_raw(raw, "hermes.coder") == "root-key"
monkeypatch.setenv("HONCHO_API_KEY", "env-key")
assert resolve_api_key_from_raw({"hosts": {"hermes.coder": {}}}, "hermes.coder") == "env-key"


class TestResolveConfigPathStickyProfile:
def test_prefers_profile_honcho_when_active_profile_set(self, tmp_path, monkeypatch):
fake_home = tmp_path / "fakehome"
fake_home.mkdir()
default_home = fake_home / ".hermes"
profile_home = default_home / "profiles" / "work"
profile_home.mkdir(parents=True)
(default_home / "honcho.json").write_text('{"apiKey": "default"}')
profile_cfg = profile_home / "honcho.json"
profile_cfg.write_text('{"baseUrl": "http://localhost:8000"}')

monkeypatch.setattr(Path, "home", lambda: fake_home)
monkeypatch.delenv("HERMES_HOME", raising=False)
monkeypatch.setattr(
"hermes_cli.profiles.get_active_profile",
lambda: "work",
)

assert resolve_config_path() == profile_cfg


class TestGetHonchoClientLocalApiKey:
def teardown_method(self):
reset_honcho_client()

@pytest.mark.skipif(
not importlib.util.find_spec("honcho"),
reason="honcho SDK not installed",
)
def test_localhost_uses_top_level_api_key(self):
fake_honcho = MagicMock(name="Honcho")
cfg = HonchoClientConfig(
api_key="jwt-from-setup",
base_url="http://localhost:8000",
workspace_id="hermes",
environment="production",
raw={"apiKey": "jwt-from-setup", "hosts": {"hermes": {}}},
)

with patch("honcho.Honcho", return_value=fake_honcho) as mock_honcho, \
patch("hermes_cli.config.load_config", return_value={}):
get_honcho_client(cfg)

assert mock_honcho.call_args.kwargs["api_key"] == "jwt-from-setup"


class TestFromGlobalConfigApiKeyInheritance:
def test_profile_host_inherits_api_key_from_hermes_block(self, tmp_path):
config_file = tmp_path / "config.json"
config_file.write_text(json.dumps({
"baseUrl": "http://localhost:8000",
"hosts": {
"hermes": {"apiKey": "shared-jwt"},
"hermes.work": {"aiPeer": "hermes.work"},
},
}))
config = HonchoClientConfig.from_global_config(
host="hermes.work", config_path=config_file,
)
assert config.api_key == "shared-jwt"


class TestGetHonchoClientBaseUrlDoublePrefixFix:
"""Regression tests for #20688 — Honcho SDK double-prefixing of /v3 for
self-hosted instances where base_url already contains a version path."""
Expand Down
20 changes: 20 additions & 0 deletions tests/honcho_plugin/test_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,26 @@ def test_alphanumeric_preserved(self):
assert mgr._sanitize_id("abc123_XYZ-789") == "abc123_XYZ-789"


class TestDialecticQueryErrors:
def test_exception_returns_honcho_error_marker(self):
mgr = HonchoSessionManager()
session = HonchoSession(
key="test",
user_peer_id="user",
assistant_peer_id="ai",
honcho_session_id="sess-1",
)
mgr._cache["test"] = session

mock_peer = MagicMock()
mock_peer.chat.side_effect = TimeoutError("Request timed out after 30.0s")
mgr._get_or_create_peer = MagicMock(return_value=mock_peer)
mgr._resolve_peer_id = MagicMock(return_value="user")

result = mgr.dialectic_query("test", "what do you know?")
assert result == "[honcho_error: TimeoutError]"


# ---------------------------------------------------------------------------
# HonchoSessionManager._format_migration_transcript
# ---------------------------------------------------------------------------
Expand Down
Loading