mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-29 06:31:32 +00:00
fix(honcho): cover pinUserPeer + aiPeer edge cases in setup, clone, and gateway cache
Three related regressions stemming from the pinUserPeer alias landing:
- Setup wizard read host-only fields when detecting current shape but the
parser supports root-level config and gives host pinUserPeer higher
precedence than pinPeerName. Re-running setup could mis-detect shape
and silently flip routing. Detection now uses the same resolver order
as HonchoClientConfig, and each shape branch scrubs every peer-mapping
key before writing so a stale pinUserPeer=false can't outrank a freshly
written pinPeerName=true. Multi no longer auto-writes
userPeerAliases={} (was silently masking root-level baselines).
- clone_honcho_for_profile inherited pinPeerName but not pinUserPeer, so
a default profile configured with the newer key produced cloned
profiles without the pin.
- Gateway cache-busting signature fingerprinted Honcho user-peer fields
but not ai_peer. Since HonchoSessionManager freezes cfg.ai_peer at
init, mid-flight aiPeer edits kept assistant writes on the old peer
until an unrelated cache eviction. ai_peer is now part of the
signature.
This commit is contained in:
parent
939499beed
commit
1a8e67076a
4 changed files with 296 additions and 30 deletions
|
|
@ -15073,20 +15073,23 @@ class GatewayRunner:
|
|||
out["tools.registry_generation"] = None
|
||||
|
||||
# Honcho identity-mapping keys live in honcho.json, not user_config.
|
||||
# HonchoSessionManager freezes the resolved peer_name / pin / aliases /
|
||||
# prefix at construction; without busting here, mid-flight honcho.json
|
||||
# edits go unread until the next unrelated cache eviction.
|
||||
# HonchoSessionManager freezes the resolved peer_name / ai_peer /
|
||||
# pin / aliases / prefix at construction; without busting here,
|
||||
# mid-flight honcho.json edits go unread until the next unrelated
|
||||
# cache eviction.
|
||||
try:
|
||||
from plugins.memory.honcho.client import HonchoClientConfig
|
||||
|
||||
hcfg = HonchoClientConfig.from_global_config()
|
||||
out["honcho.peer_name"] = hcfg.peer_name
|
||||
out["honcho.ai_peer"] = hcfg.ai_peer
|
||||
out["honcho.pin_peer_name"] = bool(hcfg.pin_peer_name)
|
||||
out["honcho.runtime_peer_prefix"] = hcfg.runtime_peer_prefix or ""
|
||||
aliases = hcfg.user_peer_aliases or {}
|
||||
out["honcho.user_peer_aliases"] = sorted(aliases.items()) if isinstance(aliases, dict) else []
|
||||
except Exception:
|
||||
out["honcho.peer_name"] = None
|
||||
out["honcho.ai_peer"] = None
|
||||
out["honcho.pin_peer_name"] = None
|
||||
out["honcho.runtime_peer_prefix"] = None
|
||||
out["honcho.user_peer_aliases"] = None
|
||||
|
|
|
|||
|
|
@ -41,17 +41,19 @@ def clone_honcho_for_profile(profile_name: str) -> bool:
|
|||
return False # already exists
|
||||
|
||||
# Clone settings from default block, override identity fields.
|
||||
# Identity-mapping keys (pinPeerName, userPeerAliases, runtimePeerPrefix)
|
||||
# carry the operator's runtime-to-peer routing intent from #27371.
|
||||
# Without them in this allowlist, a cloned profile would silently lose
|
||||
# the mapping and gateway users would resolve to raw runtime IDs,
|
||||
# fragmenting Honcho memory across an unintended set of peers.
|
||||
# Identity-mapping keys (pinPeerName/pinUserPeer, userPeerAliases,
|
||||
# runtimePeerPrefix) carry the operator's runtime-to-peer routing
|
||||
# intent from #27371. Both pin keys are inherited because
|
||||
# HonchoClientConfig prefers pinUserPeer over pinPeerName — leaving
|
||||
# the canonical key off this allowlist silently drops the pin on
|
||||
# cloned profiles when the default uses the newer name.
|
||||
new_block = {}
|
||||
for key in ("recallMode", "writeFrequency", "sessionStrategy",
|
||||
"sessionPeerPrefix", "contextTokens", "dialecticReasoningLevel",
|
||||
"dialecticDynamic", "dialecticMaxChars", "messageMaxChars",
|
||||
"dialecticMaxInputChars", "saveMessages", "observation",
|
||||
"pinPeerName", "userPeerAliases", "runtimePeerPrefix"):
|
||||
"pinPeerName", "pinUserPeer", "userPeerAliases",
|
||||
"runtimePeerPrefix"):
|
||||
val = default_block.get(key)
|
||||
if val is not None:
|
||||
new_block[key] = val
|
||||
|
|
@ -314,6 +316,72 @@ def _resolve_api_key(cfg: dict) -> str:
|
|||
return key
|
||||
|
||||
|
||||
_IDENTITY_MAPPING_KEYS = (
|
||||
"pinPeerName",
|
||||
"pinUserPeer",
|
||||
"userPeerAliases",
|
||||
"runtimePeerPrefix",
|
||||
)
|
||||
|
||||
|
||||
def _resolve_effective_identity_mapping(
|
||||
cfg: dict, hermes_host: dict
|
||||
) -> tuple[bool, dict, str, bool, bool]:
|
||||
"""Resolve the effective identity-mapping state for the active host.
|
||||
|
||||
Matches the precedence used by ``HonchoClientConfig.from_global_config``
|
||||
so the wizard reads the same shape the gateway will actually run with.
|
||||
Without this, root-level overrides and ``pinUserPeer`` (which wins over
|
||||
``pinPeerName`` at the same level) are invisible to detection, letting
|
||||
setup mis-classify the current shape and silently change effective
|
||||
routing on the next save.
|
||||
|
||||
Returns ``(pin, aliases, prefix, aliases_from_root, prefix_from_root)``.
|
||||
The ``*_from_root`` flags let the write step skip touching host keys
|
||||
whose value is actually inherited.
|
||||
"""
|
||||
pin = False
|
||||
for val in (
|
||||
hermes_host.get("pinUserPeer"),
|
||||
hermes_host.get("pinPeerName"),
|
||||
cfg.get("pinUserPeer"),
|
||||
cfg.get("pinPeerName"),
|
||||
):
|
||||
if val is not None:
|
||||
pin = bool(val)
|
||||
break
|
||||
|
||||
if "userPeerAliases" in hermes_host:
|
||||
aliases_src = hermes_host.get("userPeerAliases")
|
||||
aliases_from_root = False
|
||||
else:
|
||||
aliases_src = cfg.get("userPeerAliases")
|
||||
aliases_from_root = aliases_src is not None
|
||||
aliases = aliases_src if isinstance(aliases_src, dict) else {}
|
||||
|
||||
if "runtimePeerPrefix" in hermes_host:
|
||||
prefix_src = hermes_host.get("runtimePeerPrefix")
|
||||
prefix_from_root = False
|
||||
else:
|
||||
prefix_src = cfg.get("runtimePeerPrefix")
|
||||
prefix_from_root = prefix_src is not None
|
||||
prefix = str(prefix_src or "")
|
||||
|
||||
return pin, aliases, prefix, aliases_from_root, prefix_from_root
|
||||
|
||||
|
||||
def _scrub_identity_mapping(hermes_host: dict) -> None:
|
||||
"""Drop every peer-mapping key from the host block.
|
||||
|
||||
Called before the wizard writes a chosen shape so latent precedence
|
||||
conflicts can't survive — e.g. a stray host ``pinUserPeer: false``
|
||||
that would silently outrank a freshly written ``pinPeerName: true``
|
||||
(host ``pinUserPeer`` is first in the resolver ladder).
|
||||
"""
|
||||
for key in _IDENTITY_MAPPING_KEYS:
|
||||
hermes_host.pop(key, None)
|
||||
|
||||
|
||||
def _prompt(label: str, default: str | None = None, secret: bool = False) -> str:
|
||||
suffix = f" [{default}]" if default else ""
|
||||
sys.stdout.write(f" {label}{suffix}: ")
|
||||
|
|
@ -447,9 +515,19 @@ def cmd_setup(args) -> None:
|
|||
# shapes cover the realistic deployments; each writes a different
|
||||
# combination of pinPeerName / userPeerAliases / runtimePeerPrefix.
|
||||
# See plugins/memory/honcho/README.md for the resolver ladder.
|
||||
current_pin = bool(hermes_host.get("pinPeerName", False))
|
||||
current_aliases = hermes_host.get("userPeerAliases", {})
|
||||
current_prefix = hermes_host.get("runtimePeerPrefix", "")
|
||||
#
|
||||
# Detection must mirror the gateway resolver: root-level config and
|
||||
# ``pinUserPeer`` (which outranks ``pinPeerName`` at the same level)
|
||||
# both affect effective routing, so reading host-only fields would
|
||||
# mis-classify a profile that inherits its mapping from root or uses
|
||||
# the newer canonical key.
|
||||
(
|
||||
current_pin,
|
||||
current_aliases,
|
||||
current_prefix,
|
||||
aliases_from_root,
|
||||
prefix_from_root,
|
||||
) = _resolve_effective_identity_mapping(cfg, hermes_host)
|
||||
|
||||
if current_pin:
|
||||
current_shape = "single"
|
||||
|
|
@ -484,30 +562,52 @@ def cmd_setup(args) -> None:
|
|||
elif confirm not in {"yes", "y"}:
|
||||
new_shape = "skip"
|
||||
|
||||
# Each shape branch scrubs every peer-mapping key before writing its own,
|
||||
# so a stale ``pinUserPeer`` left behind by an earlier setup run can't
|
||||
# outrank the freshly written ``pinPeerName`` via host-level precedence.
|
||||
if new_shape == "single":
|
||||
_scrub_identity_mapping(hermes_host)
|
||||
hermes_host["pinPeerName"] = True
|
||||
hermes_host.pop("userPeerAliases", None)
|
||||
hermes_host.pop("runtimePeerPrefix", None)
|
||||
print(f" pinPeerName=true → all gateway users route to '{hermes_host.get('peerName', '?')}'.")
|
||||
elif new_shape == "multi":
|
||||
# Preserve operator-curated, host-level aliases so multi → multi
|
||||
# re-runs don't drop them. Root-sourced aliases are left to
|
||||
# cascade naturally and are NOT copied down into the host.
|
||||
prior_aliases = (
|
||||
dict(current_aliases)
|
||||
if isinstance(current_aliases, dict) and not aliases_from_root
|
||||
else {}
|
||||
)
|
||||
_scrub_identity_mapping(hermes_host)
|
||||
hermes_host["pinPeerName"] = False
|
||||
# Preserve any existing operator-curated aliases / prefix.
|
||||
if "userPeerAliases" not in hermes_host:
|
||||
hermes_host["userPeerAliases"] = {}
|
||||
# Do NOT auto-write ``userPeerAliases: {}``: an empty host map
|
||||
# would override any root-level ``userPeerAliases`` the operator
|
||||
# set as a cross-host baseline, silently disabling those aliases.
|
||||
# Absence is the right "no host opinion" signal.
|
||||
if prior_aliases:
|
||||
hermes_host["userPeerAliases"] = prior_aliases
|
||||
_prefix_default = current_prefix or ""
|
||||
_new_prefix = _prompt(
|
||||
"Runtime peer prefix (e.g. 'telegram_', blank for none)",
|
||||
default=_prefix_default,
|
||||
).strip()
|
||||
if _new_prefix:
|
||||
# Only write a host-level prefix when the operator typed one that
|
||||
# diverges from the inherited root value; otherwise let the root
|
||||
# cascade continue unmodified.
|
||||
if _new_prefix and not (prefix_from_root and _new_prefix == current_prefix):
|
||||
hermes_host["runtimePeerPrefix"] = _new_prefix
|
||||
else:
|
||||
hermes_host.pop("runtimePeerPrefix", None)
|
||||
print(" Multi-user mode: each runtime ID → own peer. Use 'hermes honcho status' to inspect.")
|
||||
elif new_shape == "hybrid":
|
||||
# Hybrid encodes operator intent at the host level: collect existing
|
||||
# entries (host or root) so the wizard never silently drops a known
|
||||
# alias, then write the combined map. Materialising root entries
|
||||
# into the host is the right move here — once the operator answers
|
||||
# the alias prompts for a host, they're declaring "this host owns
|
||||
# the mapping".
|
||||
existing_aliases = dict(current_aliases) if isinstance(current_aliases, dict) else {}
|
||||
_scrub_identity_mapping(hermes_host)
|
||||
hermes_host["pinPeerName"] = False
|
||||
peer_target = hermes_host.get("peerName") or current_peer or "user"
|
||||
existing_aliases = dict(current_aliases) if isinstance(current_aliases, dict) else {}
|
||||
print(f"\n Add runtime IDs that should alias to peer '{peer_target}'.")
|
||||
print(" Leave blank to skip a platform. Existing aliases are preserved.")
|
||||
for platform_label, alias_hint in (
|
||||
|
|
@ -521,19 +621,13 @@ def cmd_setup(args) -> None:
|
|||
existing_aliases[entered] = peer_target
|
||||
if existing_aliases:
|
||||
hermes_host["userPeerAliases"] = existing_aliases
|
||||
elif "userPeerAliases" in hermes_host:
|
||||
# No aliases entered and none pre-existing — leave the key absent.
|
||||
if not hermes_host["userPeerAliases"]:
|
||||
hermes_host.pop("userPeerAliases", None)
|
||||
_prefix_default = current_prefix or ""
|
||||
_new_prefix = _prompt(
|
||||
"Runtime peer prefix for unknown users (e.g. 'telegram_', blank for none)",
|
||||
default=_prefix_default,
|
||||
).strip()
|
||||
if _new_prefix:
|
||||
if _new_prefix and not (prefix_from_root and _new_prefix == current_prefix):
|
||||
hermes_host["runtimePeerPrefix"] = _new_prefix
|
||||
else:
|
||||
hermes_host.pop("runtimePeerPrefix", None)
|
||||
print(f" Hybrid mode: your runtime IDs → '{peer_target}', others → own peer.")
|
||||
elif new_shape == "skip":
|
||||
pass # leave config untouched
|
||||
|
|
|
|||
|
|
@ -346,7 +346,10 @@ class TestSetupWizardDeploymentShape:
|
|||
]
|
||||
host = self._run_setup(monkeypatch, tmp_path, answers=answers)
|
||||
assert host["pinPeerName"] is False
|
||||
assert host["userPeerAliases"] == {}
|
||||
# Multi must NOT auto-write ``userPeerAliases: {}``: an empty host
|
||||
# map would silently override a root-level baseline. Absence is
|
||||
# the correct "no host opinion" signal.
|
||||
assert "userPeerAliases" not in host
|
||||
assert host["runtimePeerPrefix"] == "telegram_"
|
||||
|
||||
def test_hybrid_shape_aliases_operator_runtime_ids_to_peer_name(self, monkeypatch, tmp_path):
|
||||
|
|
@ -431,5 +434,143 @@ class TestSetupWizardDeploymentShape:
|
|||
]
|
||||
host = self._run_setup(monkeypatch, tmp_path, answers=answers, initial_cfg=initial_cfg)
|
||||
assert host["pinPeerName"] is False
|
||||
assert host["userPeerAliases"] == {}
|
||||
# See test_multi_shape_leaves_pin_false_and_accepts_prefix.
|
||||
assert "userPeerAliases" not in host
|
||||
assert host["runtimePeerPrefix"] == "telegram_"
|
||||
|
||||
def test_host_pin_user_peer_true_is_detected_as_single(self, monkeypatch, tmp_path):
|
||||
"""Host-level ``pinUserPeer: true`` must classify as ``single``.
|
||||
|
||||
Pressing Enter at the shape prompt then preserves the pin instead
|
||||
of falling through to ``multi`` and orphaning the user's memory
|
||||
pool — the bug the wizard regressed when ``pinUserPeer`` landed
|
||||
as a higher-precedence alias.
|
||||
"""
|
||||
initial_cfg = {
|
||||
"apiKey": "***",
|
||||
"hosts": {"hermes": {"pinUserPeer": True, "peerName": "eri"}},
|
||||
}
|
||||
# Exhaust the iterator before the shape prompt so the scripted
|
||||
# mock falls through to the prompt's default (which is the
|
||||
# wizard-detected shape). Scripting an explicit "" would NOT
|
||||
# exercise that fallthrough — the mock returns it literally.
|
||||
answers = ["cloud", "", "eri", "hermetika", "hermes"]
|
||||
host = self._run_setup(monkeypatch, tmp_path, answers=answers, initial_cfg=initial_cfg)
|
||||
# Scrub-then-write normalises onto pinPeerName and drops the alias
|
||||
# so resolver precedence can't reintroduce ambiguity.
|
||||
assert host["pinPeerName"] is True
|
||||
assert "pinUserPeer" not in host
|
||||
|
||||
def test_host_pin_user_peer_false_overrides_root_pin_peer_name(
|
||||
self, monkeypatch, tmp_path
|
||||
):
|
||||
"""Host ``pinUserPeer: false`` outranks host ``pinPeerName`` in the
|
||||
resolver. Detection must agree, otherwise the wizard would offer
|
||||
``single`` as the default and silently re-pin a profile the
|
||||
operator explicitly unpinned via the newer key.
|
||||
"""
|
||||
initial_cfg = {
|
||||
"apiKey": "***",
|
||||
"hosts": {"hermes": {
|
||||
"pinUserPeer": False,
|
||||
"pinPeerName": True,
|
||||
"peerName": "eri",
|
||||
}},
|
||||
}
|
||||
answers = ["cloud", "", "eri", "hermetika", "hermes"]
|
||||
host = self._run_setup(monkeypatch, tmp_path, answers=answers, initial_cfg=initial_cfg)
|
||||
assert host["pinPeerName"] is False
|
||||
assert "pinUserPeer" not in host
|
||||
|
||||
def test_root_user_peer_aliases_detected_as_hybrid(self, monkeypatch, tmp_path):
|
||||
"""Root-level ``userPeerAliases`` must classify as ``hybrid`` even
|
||||
when the host block has no aliases of its own.
|
||||
"""
|
||||
initial_cfg = {
|
||||
"apiKey": "***",
|
||||
"userPeerAliases": {"86701400": "eri"},
|
||||
"hosts": {"hermes": {"peerName": "eri"}},
|
||||
}
|
||||
answers = ["cloud", "", "eri", "hermetika", "hermes"]
|
||||
host = self._run_setup(monkeypatch, tmp_path, answers=answers, initial_cfg=initial_cfg)
|
||||
assert host["pinPeerName"] is False
|
||||
# Hybrid materialises the root aliases into the host so subsequent
|
||||
# operator edits live on the host block they're inspecting.
|
||||
assert host["userPeerAliases"] == {"86701400": "eri"}
|
||||
|
||||
def test_multi_does_not_override_root_user_peer_aliases(self, monkeypatch, tmp_path):
|
||||
"""Explicit ``multi`` must leave the host ``userPeerAliases`` key
|
||||
absent, preserving any root-level aliases as a cross-host baseline.
|
||||
|
||||
Picking ``multi`` here is an active choice — detection would have
|
||||
defaulted to ``hybrid`` because root aliases exist — so the
|
||||
operator's intent is to drop the alias mapping for this host.
|
||||
We honor that by writing ``pinPeerName: false`` only, and rely
|
||||
on the host's absence of ``userPeerAliases`` to inherit root.
|
||||
That inheritance is intentional: a true wipe would require the
|
||||
operator to delete the root key explicitly.
|
||||
"""
|
||||
initial_cfg = {
|
||||
"apiKey": "***",
|
||||
"userPeerAliases": {"baseline": "eri"},
|
||||
"hosts": {"hermes": {"peerName": "eri"}},
|
||||
}
|
||||
answers = [
|
||||
"cloud", "", "eri", "hermetika", "hermes",
|
||||
"multi", # explicit multi override of detected hybrid
|
||||
]
|
||||
host = self._run_setup(monkeypatch, tmp_path, answers=answers, initial_cfg=initial_cfg)
|
||||
assert host["pinPeerName"] is False
|
||||
assert "userPeerAliases" not in host
|
||||
|
||||
def test_single_scrubs_stale_pin_user_peer_false(self, monkeypatch, tmp_path):
|
||||
"""Choosing ``single`` must drop any host-level ``pinUserPeer``,
|
||||
otherwise an existing ``pinUserPeer: false`` would outrank the
|
||||
freshly written ``pinPeerName: true`` and leave the profile
|
||||
effectively unpinned (the P1 latent-precedence regression).
|
||||
"""
|
||||
initial_cfg = {
|
||||
"apiKey": "***",
|
||||
"hosts": {"hermes": {
|
||||
"pinUserPeer": False,
|
||||
"peerName": "eri",
|
||||
}},
|
||||
}
|
||||
answers = [
|
||||
"cloud", "", "eri", "hermetika", "hermes",
|
||||
"single",
|
||||
]
|
||||
host = self._run_setup(monkeypatch, tmp_path, answers=answers, initial_cfg=initial_cfg)
|
||||
assert host["pinPeerName"] is True
|
||||
assert "pinUserPeer" not in host
|
||||
|
||||
|
||||
class TestCloneCarriesPinUserPeer:
|
||||
"""``pinUserPeer`` (canonical name for ``pinPeerName``) must survive a
|
||||
profile clone. Without this, a default profile that uses the newer
|
||||
key would silently produce cloned profiles without the pin even
|
||||
though the resolver prefers ``pinUserPeer`` over ``pinPeerName``.
|
||||
"""
|
||||
|
||||
def test_clone_inherits_host_pin_user_peer(self, monkeypatch, tmp_path):
|
||||
import plugins.memory.honcho.cli as honcho_cli
|
||||
|
||||
cfg = {
|
||||
"apiKey": "***",
|
||||
"hosts": {"hermes": {"pinUserPeer": True, "peerName": "eri"}},
|
||||
}
|
||||
cfg_path = tmp_path / "config.json"
|
||||
cfg_path.write_text("{}")
|
||||
monkeypatch.setattr(honcho_cli, "_read_config", lambda: cfg)
|
||||
monkeypatch.setattr(honcho_cli, "_config_path", lambda: cfg_path)
|
||||
monkeypatch.setattr(honcho_cli, "_local_config_path", lambda: cfg_path)
|
||||
monkeypatch.setattr(honcho_cli, "_ensure_peer_exists", lambda host_key=None: True)
|
||||
written = {}
|
||||
monkeypatch.setattr(
|
||||
honcho_cli, "_write_config", lambda c, path=None: written.setdefault("cfg", c),
|
||||
)
|
||||
|
||||
ok = honcho_cli.clone_honcho_for_profile("partner")
|
||||
assert ok is True
|
||||
new_block = written["cfg"]["hosts"]["hermes.partner"]
|
||||
assert new_block["pinUserPeer"] is True
|
||||
|
|
|
|||
|
|
@ -789,6 +789,34 @@ class TestPinTransition:
|
|||
|
||||
assert sig_no_prefix["honcho.runtime_peer_prefix"] != sig_with_prefix["honcho.runtime_peer_prefix"]
|
||||
|
||||
def test_cache_busting_signature_reflects_ai_peer(self, tmp_path, monkeypatch):
|
||||
"""Editing ``aiPeer`` mid-flight must invalidate the cached agent.
|
||||
|
||||
``HonchoSessionManager`` freezes ``cfg.ai_peer`` at construction —
|
||||
without busting here, assistant writes keep landing on the old
|
||||
peer until an unrelated cache eviction.
|
||||
"""
|
||||
from gateway.run import GatewayRunner
|
||||
|
||||
cfg_path = tmp_path / "honcho.json"
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
|
||||
cfg_path.write_text(json.dumps({
|
||||
"apiKey": "k",
|
||||
"peerName": "Igor",
|
||||
"aiPeer": "hermes",
|
||||
}))
|
||||
sig_before = GatewayRunner._extract_cache_busting_config({})
|
||||
|
||||
cfg_path.write_text(json.dumps({
|
||||
"apiKey": "k",
|
||||
"peerName": "Igor",
|
||||
"aiPeer": "hermetika",
|
||||
}))
|
||||
sig_after = GatewayRunner._extract_cache_busting_config({})
|
||||
|
||||
assert sig_before["honcho.ai_peer"] != sig_after["honcho.ai_peer"]
|
||||
|
||||
|
||||
class TestProfilePeerUniqueness:
|
||||
"""Each Hermes profile can pin to its own unique peerName.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue