Files
hermes-agent/tests/agent/test_learning_mutations.py
kshitijk4poor cd8d70bb95 test(memory): trim the journey identity tests to their invariants
AGENTS.md asks for at most two invariant tests per change and no
change-detectors; the salvage brought nine. Keep what goes red on main:

- one parametrized test: an edit/delete lands on the clicked card after
  an earlier entry was removed, and repeating it with the same id is
  refused as stale instead of hitting whatever sits at that index
  (absorbs the vanished-target test);
- the char-limit test (now also covering delete) and the BOM block in
  test_memory_writes_match_memory_tool_format (only guard for the
  shared parser);
- the render body test, minus its `count(":") == 3` id-shape assert.

Dropped: the id-layout change-detector, detail/profile/long-entry and
legacy-id variants (legacy index ids stay covered by the existing
memory:memory:N / memory:profile:2 tests). The render test also patched
a nonexistent hermes_constants._cached_default_hermes_root with
raising=False (a no-op) and set HERMES_HOME by hand; it now uses the
hermetic conftest home like its siblings. Test wording no longer claims
a memory add prepends (it appends).
2026-09-24 18:01:46 +05:30

256 lines
9.4 KiB
Python

"""Behavior contracts for journey node edit/delete (agent.learning_mutations).
Exercises the real on-disk resolution (skills dir + MEMORY.md/USER.md chunking)
against a temp HERMES_HOME, never mocks — the id→file mapping is the whole point.
"""
from __future__ import annotations
import threading
import pytest
from agent import learning_mutations as lm
from hermes_constants import get_hermes_home
_SKILL = """---
name: my-skill
description: A test skill.
---
# My Skill
Body.
"""
@pytest.fixture
def home():
base = get_hermes_home()
(base / "memories").mkdir(parents=True, exist_ok=True)
(base / "memories" / "MEMORY.md").write_text("alpha note\nline two\n§\nbeta note", encoding="utf-8")
(base / "memories" / "USER.md").write_text("user profile note", encoding="utf-8")
skill = base / "skills" / "my-skill"
skill.mkdir(parents=True, exist_ok=True)
(skill / "SKILL.md").write_text(_SKILL, encoding="utf-8")
return base
def test_parse_node_kind():
assert lm.parse_node_kind("memory:memory:0") == "memory"
assert lm.parse_node_kind("memory:profile:3") == "memory"
assert lm.parse_node_kind("debugging-hermes") == "skill"
def test_edit_memory_replaces_chunk(home):
assert lm.edit_node("memory:profile:2", "rewritten profile")["ok"]
assert (home / "memories" / "USER.md").read_text(encoding="utf-8").strip() == "rewritten profile"
def test_edit_memory_refuses_an_entry_over_the_memory_tools_limit(home):
"""Same char cap as the memory tool's replace: an over-limit entry reads as external drift to
every later mutation, so the tool's own remove/replace would refuse (with a .bak) until the
file is fixed by hand."""
from tools.memory_tool import MemoryStore
result = lm.edit_node("memory:profile:2", "x" * 5000)
assert not result["ok"] and "Shorten" in result["message"]
assert (home / "memories" / "USER.md").read_text(encoding="utf-8") == "user profile note"
assert MemoryStore().remove("user", "user profile note")["success"]
# The cap gates what an edit adds, never a delete: pruning is the way out of a file that is
# already over its total (lowered memory_char_limit, well-formed hand edits).
from tools.memory_tool import ENTRY_DELIMITER
path = home / "memories" / "USER.md"
path.write_text(ENTRY_DELIMITER.join(f"entry {i} " + "y" * 500 for i in range(4)), encoding="utf-8")
assert lm.delete_node("memory:profile:2")["ok"]
assert len(MemoryStore._read_file(path)) == 3
def test_skill_detail_returns_skill_md(home):
d = lm.node_detail("my-skill")
assert d["ok"] and d["kind"] == "skill"
assert "name: my-skill" in d["content"]
def test_delete_pinned_skill_refused(home):
from tools import skill_usage
skill_usage.set_pinned("my-skill", True)
res = lm.delete_node("my-skill")
assert not res["ok"]
assert "pinned" in res["message"]
assert (home / "skills" / "my-skill").exists()
def test_memory_writes_match_memory_tool_format(home):
"""A journey mutation must leave the file byte-identical to what the memory
tool itself writes — same §-join, no trailing-newline drift — so the two
surfaces never fight over format and indices stay aligned."""
from tools.memory_tool import ENTRY_DELIMITER, MemoryStore
assert lm.edit_node("memory:memory:0", "alpha rewritten")["ok"]
path = home / "memories" / "MEMORY.md"
entries = MemoryStore._read_file(path)
assert entries == ["alpha rewritten", "beta note"]
assert path.read_text(encoding="utf-8") == ENTRY_DELIMITER.join(entries)
# A Notepad BOM must not make the first card's id permanently "stale": the graph and the
# store must parse the file the same way.
path.write_text("alpha note\n§\nbeta note", encoding="utf-8-sig")
from agent.learning_graph import build_learning_graph
first = next(n for n in build_learning_graph()["nodes"] if n["kind"] == "memory")
assert first["label"] == "alpha note" # the BOM is not part of the card's title
assert lm.edit_node(first["id"], "alpha sans bom")["ok"]
assert MemoryStore._read_file(path) == ["alpha sans bom", "beta note"]
# ── Locking / drift (issue #119668) ─────────────────────────────────────────
# A Journey mutation shares MEMORY.md with the live agent's memory tool, so it must
# take the same lock, re-read under it and honour the drift guard — otherwise a
# memory the agent stored in the meantime is rewritten away from a stale snapshot.
def _race_memory_tool_add(monkeypatch, content: str) -> threading.Thread:
"""Start a lock-respecting ``memory_tool`` writer that appends *content* the
moment the Journey mutation resolves its node id (the read half of its
read-modify-write), then give it a moment to land. Under a correct lock the
writer blocks until the mutation has written; without one it interleaves and
the mutation's write clobbers it."""
from agent import learning_graph
from tools.memory_tool import MemoryStore
located, landed = threading.Event(), threading.Event()
real_cards = learning_graph._memory_cards
def _cards_then_let_writer_in():
cards = real_cards()
located.set()
landed.wait(timeout=0.5)
return cards
def _writer():
located.wait(timeout=5)
MemoryStore().add("memory", content)
landed.set()
monkeypatch.setattr(learning_graph, "_memory_cards", _cards_then_let_writer_in)
thread = threading.Thread(target=_writer, daemon=True)
thread.start()
return thread
def test_delete_memory_keeps_concurrent_memory_tool_add(home, monkeypatch):
from tools.memory_tool import MemoryStore
writer = _race_memory_tool_add(monkeypatch, "gamma note")
assert lm.delete_node("memory:memory:0")["ok"]
writer.join(timeout=5)
assert MemoryStore._read_file(home / "memories" / "MEMORY.md") == ["beta note", "gamma note"]
def test_edit_memory_keeps_concurrent_memory_tool_add(home, monkeypatch):
from tools.memory_tool import MemoryStore
writer = _race_memory_tool_add(monkeypatch, "gamma note")
assert lm.edit_node("memory:memory:0", "alpha rewritten")["ok"]
writer.join(timeout=5)
assert MemoryStore._read_file(home / "memories" / "MEMORY.md") == ["alpha rewritten", "beta note", "gamma note"]
@pytest.mark.parametrize("mutate", [lambda: lm.delete_node("memory:memory:0"),
lambda: lm.edit_node("memory:memory:0", "alpha rewritten")],
ids=["delete", "edit"])
def test_memory_drift_is_refused_with_backup_like_memory_tool(home, mutate):
"""Hand-edited content that wouldn't round-trip through the § parser is what
the memory tool's drift guard exists for: snapshot to .bak, refuse, leave the
file untouched. A Journey mutation must not reformat it silently."""
from tools.memory_tool_store import _drift_error
path = home / "memories" / "MEMORY.md"
raw = "alpha note\n§\n\n§\nbeta note\n"
path.write_text(raw, encoding="utf-8")
res = mutate()
assert not res["ok"]
assert path.read_text(encoding="utf-8") == raw
(backup,) = home.glob("memories/MEMORY.md.bak.*")
assert backup.read_text(encoding="utf-8") == raw
assert res["message"] == _drift_error(path, str(backup))["error"]
def test_unreadable_memory_file_is_refused_unchanged(home):
path = home / "memories" / "MEMORY.md"
path.write_bytes(b"alpha note\n\xc3\x28\n\xc2\xa7\nbeta")
res = lm.delete_node("memory:memory:0")
assert not res["ok"] and "could not be read" in res["message"]
assert path.read_bytes() == b"alpha note\n\xc3\x28\n\xc2\xa7\nbeta"
# ── Node identity: the card the user clicked, not the index it sat at ────────
def _memory_entries(home) -> list[str]:
from tools.memory_tool import MemoryStore
return MemoryStore._read_file(home / "memories" / "MEMORY.md")
def _write_memory_file(home, *entries):
from tools.memory_tool import ENTRY_DELIMITER
(home / "memories" / "MEMORY.md").write_text(ENTRY_DELIMITER.join(entries), encoding="utf-8")
def _journey_id(home, label: str) -> str:
"""The node id Journey renders for the card whose text starts with *label*."""
from agent.learning_graph import build_learning_graph
nodes = [n for n in build_learning_graph()["nodes"] if n["kind"] == "memory"]
index = next(i for i, entry in enumerate(_memory_entries(home)) if entry.startswith(label))
return nodes[index]["id"]
@pytest.mark.parametrize("op", ["edit", "delete"])
def test_mutation_targets_the_clicked_card_when_the_list_shifted_before_submit(home, op):
"""The window a displayed-index identity cannot see: an earlier entry is removed AFTER the
graph is drawn and BEFORE the edit/delete is submitted, so by the time the mutation reads the
file that index names somebody else's card."""
node_id = _journey_id(home, "beta") # what Journey showed the user
_write_memory_file(home, "beta note", "gamma note")
mutate = (lambda: lm.edit_node(node_id, "beta rewritten")) if op == "edit" else (lambda: lm.delete_node(node_id))
assert mutate()["ok"]
assert _memory_entries(home) == (["beta rewritten", "gamma note"] if op == "edit" else ["gamma note"])
# The clicked card is gone now; its id must not fall back to whatever sits at that index.
result = mutate()
assert result["ok"] is False and "stale" in result["message"]