Files
hermes-agent/agent/learning_mutations.py
TekniumandJohn Paul Soliva 26e8e3f4d5 Memory entries stay removable after the char limit is lowered (salvage #133029) (#144240)
* fix(memory): an entry written before a limit was lowered stays removable

The drift guard reads any entry longer than the whole-file char limit as
free text an external writer appended, on the premise that no tool write
can produce one. Lowering memory_char_limit or user_char_limit breaks that
premise: a well-formed file whose entry was written under the old limit
then refuses every replace and remove (tool, approval replay and the
Journey/dashboard delete alike) and writes a .bak each time, while adds are
blocked by the total. Pruning, the documented way back under the limit, is
refused, so the store stays locked until hand-edited.

A file that round-trips but carries an oversized entry now refuses only a
write that adds or rewrites text. A write that only deletes entries goes
through; when it drops an oversized entry the .bak snapshot is still taken
first and returned as drift_backup, so text glued onto an entry without a
delimiter is never deleted without a copy. Round-trip mismatches are
refused as before.

* fix(memory): a refused write beside an oversized entry still names that entry

With deletions now allowed past an oversized entry, a replace or edit that blows the budget
because of that entry came back as a plain budget refusal ("shorten the new content") with no
snapshot, although the oversized entry is the real obstacle. An over-budget refusal next to an
oversized entry returns the drift error and its .bak again; other refusals (no match, would
empty the store) pass through unchanged. The Journey edit/delete closure reports its budget
refusal through _error so the class is visible.

The Journey drift test (43d3d4e851) pinned "delete is refused beside free text"; it now pins
the edit refusal and that a delete leaving the oversized entry in place goes through, which is
the ruling for #133028 (removes must work while the store is over its limit).

* fix(memory): delete-only batches prune past a lowered cap; snapshots never overwrite or go missing

Review round (Greptile on e0ca0c027a8):
- A remove-only batch skips the final budget check like a single remove (the empty-store guard
  still applies), so pruning two small entries beside an oversized one is one call, not two.
- Snapshots are created exclusively (.bak.<ts>, then .bak.<ts>.<n>): two deletes of oversized
  entries in the same second no longer overwrite the first snapshot with content that lacks
  what the first delete removed.
- A delete that drops an oversized entry is refused when its snapshot could not be written,
  instead of deleting possibly glued free text with no copy.

* test(memory): freeze the clock so the same-second snapshot test cannot pass by luck

Greptile on 2eba9c5e78a: with the real clock, two deletes straddling a second boundary get
distinct backup names even without exclusive creation. Pinning time.time makes the collision
certain; reverting to overwrite-mode writes now fails the test.

* chore: retrigger CI (lost synchronize event)

---------

Co-authored-by: John Paul Soliva <soliva.johnpaul@icloud.com>
2026-10-25 03:00:43 -07:00

208 lines
9.9 KiB
Python

"""User-initiated edit/delete for journey nodes (learned skills + memories).
Node ids (from ``agent.learning_graph``): skills → the skill name; memories →
``memory:<source>:<index>:<fingerprint>`` (``source`` = ``memory`` for MEMORY.md /
``profile`` for USER.md; ``index`` = position in the combined card list, MEMORY.md
first; ``fingerprint`` = digest of the card's text, so the entry the user clicked is
still nameable once the list has shifted). Ids from an older graph carry no
fingerprint and resolve by position alone.
Shared by CLI ``hermes journey``, the TUI ``/journey`` overlay and the desktop.
Deleting a skill *archives* it (``hermes curator restore`` recovers it);
deleting a memory rewrites its file under the memory tool's lock.
"""
from __future__ import annotations
from pathlib import Path
from typing import Any, Callable
_MEMORY_FILES = {"memory": "MEMORY.md", "profile": "USER.md"}
_STORE_TARGETS = {"memory": "memory", "profile": "user"} # journey source -> MemoryStore target
def parse_node_kind(node_id: str) -> str:
return "memory" if node_id.startswith("memory:") else "skill"
def _parse_memory_id(node_id: str) -> tuple[str, int, str]:
"""``memory:<source>:<index>[:<fingerprint>]`` → (source, global_index, fingerprint).
The fingerprint is empty for an id minted before the graph carried one."""
parts = node_id.split(":")
try:
if len(parts) not in (3, 4) or parts[0] != "memory" or parts[1] not in _MEMORY_FILES:
raise ValueError
return parts[1], int(parts[2]), parts[3] if len(parts) == 4 else ""
except ValueError as exc:
raise ValueError(f"bad memory node id: {node_id!r}") from exc
def _resolve_fingerprint(chunks: list[str], fingerprint: str) -> int | None:
"""Index of the first entry in *chunks* whose text carries *fingerprint*, or None when gone.
The text names the entry, so a list that shifted under the user (an earlier entry removed
between the graph being drawn and the edit being submitted) still resolves to the card they
clicked. Identical entries are one entry to the memory store (it collapses byte-identical
copies on every mutation), so the first match is the entry.
"""
from agent.learning_graph import memory_fingerprint
return next((i for i, chunk in enumerate(chunks) if memory_fingerprint(chunk) == fingerprint), None)
def _locate_memory(node_id: str) -> tuple[Path, list[str], int]:
"""Resolve a memory node id to (file, all §-delimited entries, local index).
Entries come from ``MemoryStore._read_file`` — the memory tool's own parser. A
fingerprinted id resolves by the entry's text; a legacy id by position (a profile
card's local index is its global index minus the MEMORY.md card count). Read-only
view: mutations resolve the id again INSIDE ``_mutate_memory``'s lock."""
from hermes_constants import get_hermes_home
from tools.memory_tool import MemoryStore
source, gidx, fingerprint = _parse_memory_id(node_id)
path = get_hermes_home() / "memories" / _MEMORY_FILES[source]
if not path.exists():
raise ValueError(f"{path.name} not found")
chunks = MemoryStore._read_file(path)
if fingerprint:
local = _resolve_fingerprint(chunks, fingerprint)
if local is None:
raise ValueError("memory node id is stale — refresh the graph")
return path, chunks, local
from agent.learning_graph import _memory_cards
cards = _memory_cards()
if not 0 <= gidx < len(cards):
raise IndexError(f"memory index {gidx} out of range")
if cards[gidx].get("source") != source:
raise ValueError("memory node id is stale — refresh the graph")
local = gidx if source == "memory" else gidx - sum(1 for c in cards if c.get("source") == "memory")
if not 0 <= local < len(chunks):
raise ValueError("memory node id is stale — refresh the graph")
return path, chunks, local
def _mutate_memory(node_id: str, replacement: str | None) -> dict[str, Any]:
"""Replace (or, with ``replacement=None``, remove) the entry *node_id* names, through
``MemoryStore._mutate`` — the memory tool's cross-process lock, re-read under lock and
drift guard (``.bak`` snapshot + refusal when the file wouldn't round-trip). The file is
shared with the live agent, so a read-modify-write from an unlocked snapshot silently
dropped whatever the agent stored in between and reformatted hand-edited files
(#119668). The id is resolved to its entry text INSIDE the lock and matched by exact
text against the store's re-read entries; a target gone under the lock is refused."""
from tools.memory_tool import load_on_disk_store
source, _, _ = _parse_memory_id(node_id)
name = _MEMORY_FILES[source]
message = f"deleted memory from {name}" if replacement is None else f"updated memory in {name}"
def _apply(entries, limit):
from tools.memory_tool import ENTRY_DELIMITER
from tools.memory_tool_store import _error
_, chunks, local = _locate_memory(node_id)
text = chunks[local].strip()
if text not in entries:
return {"success": False, "error": "memory node id is stale — refresh the graph"}
idx = entries.index(text)
new_entries = entries[:idx] + ([] if replacement is None else [replacement]) + entries[idx + 1:]
# Same cap the memory tool enforces on replace (never on remove: deleting is how a file
# already over its total gets back under it).
if replacement is not None and (total := len(ENTRY_DELIMITER.join(new_entries))) > limit:
return _error(f"Replacement would put memory at {total:,}/{limit:,} chars. Shorten the new content.",
"over_budget")
return new_entries, message
result = load_on_disk_store()._mutate(_STORE_TARGETS[source], _apply)
if not result.get("success"):
return {"ok": False, "message": result.get("error", f"{name} write failed")}
return {"ok": True, "message": message}
def _clear_skill_cache() -> None:
try:
from agent.prompt_builder import clear_skills_system_prompt_cache
clear_skills_system_prompt_cache(clear_snapshot=True)
except Exception:
pass
def _dispatch(node_id: str, memory_fn: Callable, skill_fn: Callable, *args) -> dict[str, Any]:
try:
return (memory_fn if parse_node_kind(node_id) == "memory" else skill_fn)(node_id, *args)
except (ValueError, IndexError) as exc:
return {"ok": False, "message": str(exc)}
# ── Inspect (edit prefill) ──────────────────────────────────────────────────
def node_detail(node_id: str) -> dict[str, Any]:
"""Current content for an edit prefill. ``content`` is the full SKILL.md
(skills) or the raw memory chunk (memories)."""
return _dispatch(node_id, _memory_detail, _skill_detail)
def _memory_detail(node_id: str) -> dict[str, Any]:
_, chunks, local = _locate_memory(node_id)
body = chunks[local].strip()
return {"ok": True, "kind": "memory", "id": node_id, "label": body.splitlines()[0][:80], "content": body}
def _skill_detail(node_id: str) -> dict[str, Any]:
from tools.skill_manager_tool import _find_skill
found = _find_skill(node_id)
if not found:
return {"ok": False, "message": f"skill '{node_id}' not found"}
skill_md = Path(found["path"]) / "SKILL.md"
if not skill_md.exists():
return {"ok": False, "message": f"SKILL.md missing for '{node_id}'"}
return {"ok": True, "kind": "skill", "id": node_id, "label": node_id, "content": skill_md.read_text(encoding="utf-8-sig")}
# ── Delete ──────────────────────────────────────────────────────────────────
def delete_node(node_id: str) -> dict[str, Any]:
return _dispatch(node_id, _delete_memory, _delete_skill)
def _delete_skill(name: str) -> dict[str, Any]:
from tools import skill_usage
# Pin must be respected by autonomous maintenance. The curator already skips pinned skills from every
# auto-transition; the background review fork is the same kind of autonomous, no-user-present actor, so
# it must not write to a pinned skill either (issue #25839). This is stricter than the foreground
# ``_pinned_guard`` (which only blocks deletion) precisely because there is no user in the loop to
# consent to an edit here.
if skill_usage.get_record(name).get("pinned"):
return {"ok": False, "message": f"'{name}' is pinned — unpin it first (hermes curator unpin {name})"}
ok, message = skill_usage.archive_skill(name)
if ok:
_clear_skill_cache()
return {"ok": ok, "message": f"archived '{name}' — restore with: hermes curator restore {name}" if ok else message}
def _delete_memory(node_id: str) -> dict[str, Any]:
return _mutate_memory(node_id, None)
# ── Edit ────────────────────────────────────────────────────────────────────
def edit_node(node_id: str, content: str) -> dict[str, Any]:
return _dispatch(node_id, _edit_memory, _edit_skill, content)
def _edit_skill(name: str, content: str) -> dict[str, Any]:
from tools.skill_manager_tool import _edit_skill as _do_edit
result = _do_edit(name, content)
if result.get("success"):
_clear_skill_cache()
return {"ok": True, "message": f"updated '{name}'"}
return {"ok": False, "message": result.get("error", "edit failed")}
def _edit_memory(node_id: str, content: str) -> dict[str, Any]:
_parse_memory_id(node_id) # id errors win over the empty-body message
body = content.strip()
if not body:
return {"ok": False, "message": "empty memory — use delete to remove it"}
return _mutate_memory(node_id, body)