mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-17 09:41:58 +00:00
refactor(honcho): canonicalize identity-mapping on pinUserPeer, migrate legacy key
The setup wizard wrote the legacy pinPeerName even though pinUserPeer is the canonical key that outranks it in the resolver — so it had to scrub the canonical key afterward to stop it winning. Write pinUserPeer directly and migrate any legacy pinPeerName onto it on touch (setup load + clone), which removes the precedence-fighting entirely. Resolver still reads pinPeerName as a back-compat alias; that's deferred.
This commit is contained in:
Erosika
parent
c4811c382f
commit
bb5cb32838
2 changed files with 81 additions and 42 deletions
|
|
@ -41,22 +41,20 @@ 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/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.
|
||||
# Identity-mapping keys (pinUserPeer, userPeerAliases, runtimePeerPrefix)
|
||||
# carry the operator's runtime-to-peer routing intent from #27371.
|
||||
new_block = {}
|
||||
for key in ("recallMode", "writeFrequency", "sessionStrategy",
|
||||
"sessionPeerPrefix", "contextTokens", "dialecticReasoningLevel",
|
||||
"dialecticDynamic", "dialecticMaxChars", "messageMaxChars",
|
||||
"dialecticMaxInputChars", "saveMessages", "observation",
|
||||
"pinPeerName", "pinUserPeer", "userPeerAliases",
|
||||
"runtimePeerPrefix"):
|
||||
"pinUserPeer", "userPeerAliases", "runtimePeerPrefix"):
|
||||
val = default_block.get(key)
|
||||
if val is not None:
|
||||
new_block[key] = val
|
||||
# Carry a legacy default-block pinPeerName forward under the canonical key.
|
||||
if "pinUserPeer" not in new_block and default_block.get("pinPeerName") is not None:
|
||||
new_block["pinUserPeer"] = default_block["pinPeerName"]
|
||||
|
||||
# Inherit peer name from default
|
||||
peer_name = default_block.get("peerName") or cfg.get("peerName")
|
||||
|
|
@ -371,15 +369,28 @@ def _resolve_effective_identity_mapping(
|
|||
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).
|
||||
Called before the wizard writes a chosen shape so a stale alias, prefix,
|
||||
or pin from an earlier run can't bleed into the new mapping.
|
||||
"""
|
||||
for key in _IDENTITY_MAPPING_KEYS:
|
||||
hermes_host.pop(key, None)
|
||||
|
||||
|
||||
def _migrate_pin_key(block: dict) -> bool:
|
||||
"""Rewrite a legacy ``pinPeerName`` to canonical ``pinUserPeer`` in place.
|
||||
|
||||
``pinUserPeer`` wins over ``pinPeerName`` in the resolver, so setup writes
|
||||
only the canonical form and migrates on touch to stop configs carrying
|
||||
both. Returns True if the block changed.
|
||||
"""
|
||||
if "pinPeerName" not in block:
|
||||
return False
|
||||
legacy = block.pop("pinPeerName")
|
||||
if "pinUserPeer" not in block:
|
||||
block["pinUserPeer"] = legacy
|
||||
return True
|
||||
|
||||
|
||||
def _prompt(label: str, default: str | None = None, secret: bool = False) -> str:
|
||||
suffix = f" [{default}]" if default else ""
|
||||
sys.stdout.write(f" {label}{suffix}: ")
|
||||
|
|
@ -446,6 +457,10 @@ def cmd_setup(args) -> None:
|
|||
hosts = cfg.setdefault("hosts", {})
|
||||
hermes_host = hosts.setdefault(_host_key(), {})
|
||||
|
||||
# Canonicalize any legacy pinPeerName before detection/writes.
|
||||
_migrate_pin_key(cfg)
|
||||
_migrate_pin_key(hermes_host)
|
||||
|
||||
# --- 1. Cloud or local? ---
|
||||
print(" Deployment:")
|
||||
print(" cloud -- Honcho cloud (api.honcho.dev)")
|
||||
|
|
@ -599,12 +614,11 @@ def cmd_setup(args) -> None:
|
|||
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.
|
||||
# so a stale alias/prefix/pin from an earlier run starts clean.
|
||||
if new_shape == "single":
|
||||
_scrub_identity_mapping(hermes_host)
|
||||
hermes_host["pinPeerName"] = True
|
||||
print(f" pinPeerName=true → all gateway users route to '{hermes_host.get('peerName', '?')}'.")
|
||||
hermes_host["pinUserPeer"] = True
|
||||
print(f" pinUserPeer=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
|
||||
|
|
@ -615,7 +629,7 @@ def cmd_setup(args) -> None:
|
|||
else {}
|
||||
)
|
||||
_scrub_identity_mapping(hermes_host)
|
||||
hermes_host["pinPeerName"] = False
|
||||
hermes_host["pinUserPeer"] = False
|
||||
# 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.
|
||||
|
|
@ -642,7 +656,7 @@ def cmd_setup(args) -> None:
|
|||
# the mapping".
|
||||
existing_aliases = dict(current_aliases) if isinstance(current_aliases, dict) else {}
|
||||
_scrub_identity_mapping(hermes_host)
|
||||
hermes_host["pinPeerName"] = False
|
||||
hermes_host["pinUserPeer"] = False
|
||||
peer_target = hermes_host.get("peerName") or current_peer or "user"
|
||||
print(f"\n Add runtime IDs that should alias to peer '{peer_target}'.")
|
||||
print(" Leave blank to skip a platform. Existing aliases are preserved.")
|
||||
|
|
|
|||
|
|
@ -239,7 +239,7 @@ class TestCloneHonchoForProfile:
|
|||
"""Identity-key carryover during profile cloning.
|
||||
|
||||
The host-scoped identity-mapping keys (``userPeerAliases``,
|
||||
``runtimePeerPrefix``, ``pinPeerName``) must survive a clone; otherwise
|
||||
``runtimePeerPrefix``, ``pinUserPeer``) must survive a clone; otherwise
|
||||
the new profile silently fragments memory by resolving gateway users to
|
||||
raw runtime IDs instead of operator-declared peers.
|
||||
"""
|
||||
|
|
@ -290,7 +290,7 @@ class TestCloneHonchoForProfile:
|
|||
new_block = written["cfg"]["hosts"]["hermes_coder"]
|
||||
assert new_block["runtimePeerPrefix"] == "telegram_"
|
||||
|
||||
def test_pin_peer_name_carries_into_cloned_profile(self, monkeypatch, tmp_path):
|
||||
def test_legacy_pin_peer_name_migrates_to_canonical_on_clone(self, monkeypatch, tmp_path):
|
||||
cfg = {
|
||||
"apiKey": "***",
|
||||
"hosts": {
|
||||
|
|
@ -304,7 +304,8 @@ class TestCloneHonchoForProfile:
|
|||
ok = honcho_cli.clone_honcho_for_profile("coder")
|
||||
assert ok is True
|
||||
new_block = written["cfg"]["hosts"]["hermes_coder"]
|
||||
assert new_block["pinPeerName"] is True
|
||||
assert new_block["pinUserPeer"] is True
|
||||
assert "pinPeerName" not in new_block
|
||||
|
||||
def test_unset_identity_keys_do_not_appear_in_cloned_profile(self, monkeypatch, tmp_path):
|
||||
cfg = {
|
||||
|
|
@ -317,6 +318,7 @@ class TestCloneHonchoForProfile:
|
|||
new_block = written["cfg"]["hosts"]["hermes_coder"]
|
||||
assert "userPeerAliases" not in new_block
|
||||
assert "runtimePeerPrefix" not in new_block
|
||||
assert "pinUserPeer" not in new_block
|
||||
assert "pinPeerName" not in new_block
|
||||
|
||||
|
||||
|
|
@ -409,7 +411,7 @@ class TestSetupWizardDeploymentShape:
|
|||
}},
|
||||
}
|
||||
host = self._run_setup(monkeypatch, tmp_path, answers=answers, initial_cfg=initial_cfg)
|
||||
assert host["pinPeerName"] is True
|
||||
assert host["pinUserPeer"] is True
|
||||
assert "userPeerAliases" not in host
|
||||
assert "runtimePeerPrefix" not in host
|
||||
|
||||
|
|
@ -424,7 +426,7 @@ class TestSetupWizardDeploymentShape:
|
|||
"telegram_", # runtime peer prefix
|
||||
]
|
||||
host = self._run_setup(monkeypatch, tmp_path, answers=answers)
|
||||
assert host["pinPeerName"] is False
|
||||
assert host["pinUserPeer"] is False
|
||||
# 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.
|
||||
|
|
@ -446,7 +448,7 @@ class TestSetupWizardDeploymentShape:
|
|||
"", # runtime peer prefix (skip)
|
||||
]
|
||||
host = self._run_setup(monkeypatch, tmp_path, answers=answers)
|
||||
assert host["pinPeerName"] is False
|
||||
assert host["pinUserPeer"] is False
|
||||
assert host["userPeerAliases"] == {
|
||||
"86701400": "eri",
|
||||
"491827364": "eri",
|
||||
|
|
@ -454,6 +456,8 @@ class TestSetupWizardDeploymentShape:
|
|||
assert "runtimePeerPrefix" not in host
|
||||
|
||||
def test_skip_shape_preserves_existing_identity_config(self, monkeypatch, tmp_path):
|
||||
# Seeds the legacy ``pinPeerName``: skip must leave the mapping intact
|
||||
# except for the on-load migration onto the canonical key.
|
||||
initial_cfg = {
|
||||
"apiKey": "***",
|
||||
"hosts": {"hermes": {
|
||||
|
|
@ -466,7 +470,8 @@ class TestSetupWizardDeploymentShape:
|
|||
"cloud", "", "eri", "hermetika", "hermes", "skip",
|
||||
]
|
||||
host = self._run_setup(monkeypatch, tmp_path, answers=answers, initial_cfg=initial_cfg)
|
||||
assert host["pinPeerName"] is True
|
||||
assert host["pinUserPeer"] is True
|
||||
assert "pinPeerName" not in host
|
||||
assert host["userPeerAliases"] == {"keep": "me"}
|
||||
assert host["runtimePeerPrefix"] == "keep_"
|
||||
|
||||
|
|
@ -494,7 +499,7 @@ class TestSetupWizardDeploymentShape:
|
|||
"", # runtime prefix (skip)
|
||||
]
|
||||
host = self._run_setup(monkeypatch, tmp_path, answers=answers, initial_cfg=initial_cfg)
|
||||
assert host["pinPeerName"] is False
|
||||
assert host["pinUserPeer"] is False
|
||||
assert host["userPeerAliases"] == {"86701400": "eri"}
|
||||
|
||||
def test_single_to_multi_yes_override_keeps_multi(self, monkeypatch, tmp_path):
|
||||
|
|
@ -512,7 +517,7 @@ class TestSetupWizardDeploymentShape:
|
|||
"telegram_", # runtime peer prefix
|
||||
]
|
||||
host = self._run_setup(monkeypatch, tmp_path, answers=answers, initial_cfg=initial_cfg)
|
||||
assert host["pinPeerName"] is False
|
||||
assert host["pinUserPeer"] is False
|
||||
# See test_multi_shape_leaves_pin_false_and_accepts_prefix.
|
||||
assert "userPeerAliases" not in host
|
||||
assert host["runtimePeerPrefix"] == "telegram_"
|
||||
|
|
@ -535,10 +540,9 @@ class TestSetupWizardDeploymentShape:
|
|||
# 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
|
||||
# Scrub-then-write normalises onto the canonical pinUserPeer.
|
||||
assert host["pinUserPeer"] is True
|
||||
assert "pinPeerName" not in host
|
||||
|
||||
def test_host_pin_user_peer_false_overrides_root_pin_peer_name(
|
||||
self, monkeypatch, tmp_path
|
||||
|
|
@ -558,8 +562,8 @@ class TestSetupWizardDeploymentShape:
|
|||
}
|
||||
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
|
||||
assert host["pinUserPeer"] is False
|
||||
assert "pinPeerName" not in host
|
||||
|
||||
def test_root_user_peer_aliases_detected_as_hybrid(self, monkeypatch, tmp_path):
|
||||
"""Root-level ``userPeerAliases`` must classify as ``hybrid`` even
|
||||
|
|
@ -572,7 +576,7 @@ class TestSetupWizardDeploymentShape:
|
|||
}
|
||||
answers = ["cloud", "", "eri", "hermetika", "hermes"]
|
||||
host = self._run_setup(monkeypatch, tmp_path, answers=answers, initial_cfg=initial_cfg)
|
||||
assert host["pinPeerName"] is False
|
||||
assert host["pinUserPeer"] 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"}
|
||||
|
|
@ -584,7 +588,7 @@ class TestSetupWizardDeploymentShape:
|
|||
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
|
||||
We honor that by writing ``pinUserPeer: 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.
|
||||
|
|
@ -599,14 +603,12 @@ class TestSetupWizardDeploymentShape:
|
|||
"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 host["pinUserPeer"] 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).
|
||||
"""Choosing ``single`` must overwrite a stale ``pinUserPeer: false``
|
||||
with ``pinUserPeer: true`` so the profile ends up genuinely pinned.
|
||||
"""
|
||||
initial_cfg = {
|
||||
"apiKey": "***",
|
||||
|
|
@ -620,8 +622,7 @@ class TestSetupWizardDeploymentShape:
|
|||
"single",
|
||||
]
|
||||
host = self._run_setup(monkeypatch, tmp_path, answers=answers, initial_cfg=initial_cfg)
|
||||
assert host["pinPeerName"] is True
|
||||
assert "pinUserPeer" not in host
|
||||
assert host["pinUserPeer"] is True
|
||||
|
||||
|
||||
class TestCloneCarriesPinUserPeer:
|
||||
|
|
@ -653,3 +654,27 @@ class TestCloneCarriesPinUserPeer:
|
|||
assert ok is True
|
||||
new_block = written["cfg"]["hosts"]["hermes_partner"]
|
||||
assert new_block["pinUserPeer"] is True
|
||||
|
||||
|
||||
class TestMigratePinKey:
|
||||
"""``_migrate_pin_key`` rewrites the legacy ``pinPeerName`` onto the
|
||||
canonical ``pinUserPeer`` in place, without clobbering an existing
|
||||
canonical value."""
|
||||
|
||||
def test_legacy_key_renamed_to_canonical(self):
|
||||
import plugins.memory.honcho.cli as honcho_cli
|
||||
block = {"pinPeerName": True}
|
||||
assert honcho_cli._migrate_pin_key(block) is True
|
||||
assert block == {"pinUserPeer": True}
|
||||
|
||||
def test_canonical_key_wins_when_both_present(self):
|
||||
import plugins.memory.honcho.cli as honcho_cli
|
||||
block = {"pinPeerName": True, "pinUserPeer": False}
|
||||
assert honcho_cli._migrate_pin_key(block) is True
|
||||
assert block == {"pinUserPeer": False}
|
||||
|
||||
def test_noop_when_no_legacy_key(self):
|
||||
import plugins.memory.honcho.cli as honcho_cli
|
||||
block = {"pinUserPeer": True}
|
||||
assert honcho_cli._migrate_pin_key(block) is False
|
||||
assert block == {"pinUserPeer": True}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue