fix(honcho): harden self-hosted setup paths
Self-hosted Honcho setup had four sharp edges: - local/cloud URLs ending in /vN double-prefixed by the SDK (/v3/v3/... 404) - authenticated local servers had no setup prompt for a JWT/bearer token - profile-derived host keys could be dot-containing workspace IDs Honcho rejects - memory-provider config files with API keys written world-readable per umask This keeps existing behavior but makes those paths safer: - strip a trailing /vN version segment from any configured baseUrl before SDK init (the SDK's route builders always prepend their own version prefix); auth-skipping stays loopback-only - add an optional local JWT/bearer prompt in honcho setup, stored under hosts.<host>.apiKey - derive new profile host keys with underscores, still reading legacy hermes.<profile> blocks - write memory-provider config files atomically with 0600 via a shared utils.atomic_json_write(mode=) arg (honcho/hindsight/mem0/supermemory) - skip honcho.json parsing in gateway cache-busting unless Honcho is the active memory provider; memoize by honcho.json mtime when active - bust the gateway agent cache on memory.provider change - add a hermes memory setup <provider> one-liner so fresh installs can configure a named provider without the picker (the per-provider hermes <provider> subcommand only registers once that provider is active) Closes #20688, #29885, #26459, #30246, #33382, #32244. Co-authored-by: BROCCOLO1D
This commit is contained in:
committed by
kshitij
co-authored by
BROCCOLO1D
parent
aa32edcac5
commit
827ce602db
@@ -0,0 +1,50 @@
|
||||
"""Tests for `hermes memory setup [provider]` routing.
|
||||
|
||||
The `memory setup` subcommand accepts an optional positional ``provider`` so a
|
||||
fresh install can configure a specific provider directly (e.g.
|
||||
``hermes memory setup honcho``) without the interactive picker — which matters
|
||||
because the per-provider ``hermes <provider>`` subcommand is only registered
|
||||
once that provider is active.
|
||||
"""
|
||||
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import patch
|
||||
|
||||
from hermes_cli import memory_setup
|
||||
|
||||
|
||||
class TestMemorySetupProviderRouting:
|
||||
def test_setup_with_provider_arg_skips_picker(self):
|
||||
"""`memory setup honcho` routes straight to cmd_setup_provider."""
|
||||
args = SimpleNamespace(memory_command="setup", provider="honcho")
|
||||
with patch.object(memory_setup, "cmd_setup_provider") as direct, \
|
||||
patch.object(memory_setup, "cmd_setup") as picker:
|
||||
memory_setup.memory_command(args)
|
||||
direct.assert_called_once_with("honcho")
|
||||
picker.assert_not_called()
|
||||
|
||||
def test_setup_without_provider_runs_picker(self):
|
||||
"""`memory setup` (no provider) runs the interactive picker."""
|
||||
args = SimpleNamespace(memory_command="setup", provider=None)
|
||||
with patch.object(memory_setup, "cmd_setup_provider") as direct, \
|
||||
patch.object(memory_setup, "cmd_setup") as picker:
|
||||
memory_setup.memory_command(args)
|
||||
picker.assert_called_once_with(args)
|
||||
direct.assert_not_called()
|
||||
|
||||
def test_setup_with_missing_provider_attr_runs_picker(self):
|
||||
"""A SimpleNamespace lacking `provider` must not crash — fall back to picker."""
|
||||
args = SimpleNamespace(memory_command="setup")
|
||||
with patch.object(memory_setup, "cmd_setup_provider") as direct, \
|
||||
patch.object(memory_setup, "cmd_setup") as picker:
|
||||
memory_setup.memory_command(args)
|
||||
picker.assert_called_once_with(args)
|
||||
direct.assert_not_called()
|
||||
|
||||
def test_unknown_provider_reports_and_returns_early(self, capsys):
|
||||
"""An unknown provider name surfaces a helpful message and returns
|
||||
before any config load/save (the not-found guard precedes those imports)."""
|
||||
memory_setup.cmd_setup_provider("notaprovider")
|
||||
out = capsys.readouterr().out
|
||||
assert "not found" in out
|
||||
assert "hermes memory setup" in out
|
||||
@@ -754,8 +754,8 @@ class TestRenameProfile:
|
||||
|
||||
cfg = json.loads(honcho_path.read_text())
|
||||
assert "hermes.ssi_health" not in cfg["hosts"]
|
||||
assert cfg["hosts"]["hermes.heimdall"]["aiPeer"] == "ssi_health"
|
||||
assert cfg["hosts"]["hermes.heimdall"]["peerName"] == "user-peer"
|
||||
assert cfg["hosts"]["hermes_heimdall"]["aiPeer"] == "ssi_health"
|
||||
assert cfg["hosts"]["hermes_heimdall"]["peerName"] == "user-peer"
|
||||
|
||||
def test_pins_ai_peer_when_absent_on_honcho_host_rename(self, profile_env):
|
||||
tmp_path = profile_env
|
||||
@@ -772,8 +772,8 @@ class TestRenameProfile:
|
||||
|
||||
cfg = json.loads(honcho_path.read_text())
|
||||
assert "hermes.ssi_health" not in cfg["hosts"]
|
||||
assert cfg["hosts"]["hermes.heimdall"]["aiPeer"] == "ssi_health"
|
||||
assert cfg["hosts"]["hermes.heimdall"]["workspace"] == "hermes"
|
||||
assert cfg["hosts"]["hermes_heimdall"]["aiPeer"] == "ssi_health"
|
||||
assert cfg["hosts"]["hermes_heimdall"]["workspace"] == "hermes"
|
||||
|
||||
def test_does_not_overwrite_existing_honcho_host_on_rename(self, profile_env):
|
||||
tmp_path = profile_env
|
||||
@@ -782,7 +782,7 @@ class TestRenameProfile:
|
||||
honcho_path.write_text(json.dumps({
|
||||
"hosts": {
|
||||
"hermes.ssi_health": {"aiPeer": "ssi_health"},
|
||||
"hermes.heimdall": {"aiPeer": "heimdall"},
|
||||
"hermes_heimdall": {"aiPeer": "heimdall"},
|
||||
}
|
||||
}))
|
||||
|
||||
@@ -791,7 +791,7 @@ class TestRenameProfile:
|
||||
|
||||
cfg = json.loads(honcho_path.read_text())
|
||||
assert cfg["hosts"]["hermes.ssi_health"]["aiPeer"] == "ssi_health"
|
||||
assert cfg["hosts"]["hermes.heimdall"]["aiPeer"] == "heimdall"
|
||||
assert cfg["hosts"]["hermes_heimdall"]["aiPeer"] == "heimdall"
|
||||
|
||||
def test_default_raises_value_error(self, profile_env):
|
||||
with pytest.raises(ValueError, match="default"):
|
||||
|
||||
Reference in New Issue
Block a user