hermes-admin/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 4 Packages Projects Releases Wiki Activity

fix(memory): stop false drift refusals; refused writes name the exact fix

Browse Source 
Fleet telemetry (14d, v0.21.5) shows the builtin store returning
success=False on 34-47% of add/replace/remove calls. Live `hermes chat -q`
runs against origin/main reproduced the dominant causes:

- Drift guard refused every replace/remove/batch on a file whose only
  difference from the tool's own layout was whitespace (blank lines
  around the delimiter, an empty entry). Parsing only strips whitespace,
  so flushing such a file loses nothing; the round-trip signal could not
  protect any content and turned the store read-only until hand-fixed
  (#107270). The guard now fires only on the signal that can actually
  lose foreign text: one entry larger than the whole store's limit.
- An over-budget add/replace/batch said "would exceed the limit" but not
  by how much, so models retried with consolidations that were still too
  big. Every over-budget message now states the exact chars to free and
  points at the one-batch fix.
- An old_text that matched nothing (paraphrased, stale after a concurrent
  write, or several entries glued with the delimiter) returned no hint on
  the batch path. Zero-match errors now carry `closest_entries` (top word
  overlap) on both paths, and flag a delimiter-spanning old_text.
- A batch replace missing its own `content` now says to set it on the
  operation object.

Fixes #107270. Same diagnosis as #107278 (@KoNit-K) and #61612 (@webtecnica),
which normalized the round-trip comparison instead of dropping it.
This commit is contained in:
Teknium
2026-10-04 18:56:06 -07:00
parent 2983c25373
commit 8079fd01f4
4 changed files with 94 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
tests/agent/test_learning_mutations.py
View File

@@ -186,13 +186,13 @@ def test_edit_memory_keeps_concurrent_memory_tool_add(home, monkeypatch):
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."""
"""Free-form text appended without § delimiters (one "entry" past the whole-store
limit) is what the memory tool's drift guard exists for: snapshot to .bak, refuse,
leave the file untouched. A Journey mutation must not flatten it silently."""
from tools.memory_tool_store import _drift_error
path = home / "memories" / "MEMORY.md"
raw = "alpha note\n§\n\n§\nbeta note\n"
raw = "alpha note\n§\nbeta note\n\n## Vendor Master\n" + "x" * 2300 + "\n"
path.write_text(raw, encoding="utf-8")
res = mutate()

26
tests/tools/test_memory_tool.py
View File

@@ -253,12 +253,26 @@ class TestMemoryStoreReplace:
store.add("memory", 'User runs a fleet (\u201cOmarchy\u201d \u2014 Trinity)')
store.add("memory", "Tests: run \u2018make test\u2019 (needs\n docker up)")
assert store.replace("memory", "fleet (\"Omarchy\" - Trinity)", "fleet (Zeus hub)")["success"] is True
assert store.apply_batch("memory", [{"action": "replace", "old_text": "run 'make test' (needs docker up)",
assert store.apply_batch("memory", [{"action": "replace", "old_text": "run 'make test' (needs\\n docker up)",
"content": "Tests: make test-fast"}])["success"] is True
assert store.memory_entries == ["fleet (Zeus hub)", "Tests: make test-fast"]
result = store.replace("memory", "fleet 'on-prem'", "x")
assert result["success"] is False and "No entry matched" in result["error"]
class TestSelfCorrectingFailures:
def test_failures_name_the_fix(self, store):
"""A refused write tells the model exactly what to change: the chars to free, and
for an old_text that matches nothing, the entries it most resembles."""
store.add("memory", "Deploys go through GitHub Actions to Fly.io with manual approval.")
store.add("memory", "x" * 380) # 65 + 3 + 380 = 448 of 500
full = store.add("memory", "y" * 54) # 448 + 3 + 54 = 505
assert "by 5 chars" in full["error"] and "free at least 5 chars" in full["error"]
miss = store.apply_batch("memory", [{"action": "replace", "old_text": "Deploys via GitHub Actions to Fly",
"content": "Deploys: Fly.io + canary."}])
assert miss["closest_entries"] == ["Deploys go through GitHub Actions to Fly.io with manual approval."]
assert "current_entries" not in miss # batch aborts still never echo the store (#97316)
class TestMemoryStoreRemove:
def test_remove_entry(self, store):
store.add("memory", "temporary note")
@@ -676,6 +690,16 @@ class TestExternalDriftGuard:
result = store.replace("memory", "Entry two", "Entry two replaced.")
assert result["success"] is True
def test_hand_formatted_layout_is_not_drift(self, store):
"""#107270: blank lines around § and an empty entry lose nothing on a flush (parsing
only strips whitespace), so replace/remove must work and leave no .bak behind."""
path = store._path_for("memory")
path.write_text("Deploys: Fly.io, manual approval.\n\n§\n\nHost: forge\n§\n\n§\nRepo: acme", encoding="utf-8")
assert store.replace("memory", "Deploys:", "Deploys: Fly.io, 30m canary, manual approval.")["success"] is True
assert store.remove("memory", "Repo: acme")["success"] is True
assert path.read_text(encoding="utf-8") == "Deploys: Fly.io, 30m canary, manual approval.\n§\nHost: forge"
assert not list(path.parent.glob("MEMORY.md.bak.*"))
def test_drift_guard_also_protects_user_target(self, store):
"""USER.md gets the same guarantee as MEMORY.md."""
store.add("user", "Some preference.")

93
tools/memory_tool_store.py
View File

@@ -5,6 +5,7 @@ in ``tools.memory_tool`` and is read lazily."""
import logging
import os
import re
import time
from contextlib import contextmanager, suppress
from pathlib import Path
@@ -36,9 +37,9 @@ def _error(message: str, **extra) -> Dict[str, Any]:
def _drift_error(path: Path, bak_path: str) -> Dict[str, Any]:
"""External drift: the file wouldn't round-trip, so flushing would discard content."""
return _error((
f"Refusing to write {path.name}: file on disk has content that wouldn't round-trip "
f"through the memory tool (likely added by the patch tool, a shell append, a manual edit, "
f"or a concurrent session). A snapshot was saved to {bak_path}. Resolve the drift first — "
f"Refusing to write {path.name}: it holds an entry larger than the whole store's char limit, "
f"i.e. free-form text written without § delimiters (the patch tool, a shell append, a manual "
f"edit). Rewriting it would lose that text. A snapshot was saved to {bak_path}. Resolve the drift first — "
f"either rewrite the file as a clean §-delimited list of entries, or move the extra "
f"content out — then retry. This guard exists to prevent silent data loss (issue #26045)."
), drift_backup=bak_path, remediation=(
@@ -63,9 +64,10 @@ _MATCH_FOLD = str.maketrans({**dict.fromkeys("\"`\u2018\u2019\u201a\u201b\u201c\
def _normalize_for_match(text: str) -> str:
"""Quote/dash/whitespace-folded form for tolerant old_text matching. Models re-emit an
entry with straight quotes for curly ones, '-' for an em dash, or unwrapped lines, and a
byte-exact substring test then reports "No entry matched" for an unambiguous target."""
return " ".join(str(text).translate(_MATCH_FOLD).split())
entry with straight quotes for curly ones, '-' for an em dash, unwrapped lines, or a
newline over-escaped to a literal backslash-n, and a byte-exact substring test then
reports "No entry matched" for an unambiguous target."""
return " ".join(str(text).replace("\\n", " ").translate(_MATCH_FOLD).split())
def _substring_matches(entries: List[str], old_text: str) -> List[int]:
@@ -92,6 +94,27 @@ def _find_unique_match(entries: List[str], old_text: str) -> Tuple[Optional[int]
return (matches[0] if matches else None), False
def _no_match_error(entries: List[str], old_text: str, verb: str, **extra) -> Dict[str, Any]:
"""Zero-match error that names the fix: the entries sharing the most words with
*old_text* (a paraphrased or stale old_text usually still shares most of them), and a
pointer when old_text spans several entries joined by the delimiter."""
def _words(text: str) -> set:
return set(re.findall(r"\w+", text.lower()))
words = _words(old_text)
scored = sorted(((len(words & _words(e)) / len(words), i) for i, e in enumerate(entries)) if words else [],
key=lambda p: (-p[0], p[1]))
closest = [entries[i] for score, i in scored[:3] if score >= 0.3]
hint = (" old_text must come from ONE entry ('§' separates entries); use one op per entry."
if "§" in old_text else "")
return _error(f"No entry matched '{old_text}'.{hint} Retry the {verb} with old_text copied verbatim "
f"from the intended entry" + (" (closest_entries below)." if closest else "."),
**({"closest_entries": closest} if closest else {}), **extra)
def _over_limit(new_total: int, limit: int) -> str:
return f"would put memory at {new_total:,}/{limit:,} chars: free at least {new_total - limit:,} chars"
def _pinned_index(entries: List[str], matched_entry: str) -> Optional[int]:
"""Index of the exact entry a staged write was reviewed against; None once it is gone (stale)."""
return entries.index(matched_entry) if matched_entry in entries else None
@@ -258,12 +281,14 @@ class MemoryStore:
return self._consolidation_failure(
_error(message, current_entries=self._entries_for(target), usage=self._usage(target)))
def _batch_failure(self, target: str, message: str) -> Dict[str, Any]:
def _batch_failure(self, target: str, message: str, closest_entries: Optional[List[str]] = None) -> Dict[str, Any]:
"""Batch-abort failure WITHOUT ``current_entries``: the store did not change and the
caller already holds the inventory, so echoing it made each consolidation retry
grow the context it was invoked to shrink (#97316)."""
grow the context it was invoked to shrink (#97316). A zero-match op carries only
its few ``closest_entries``."""
return self._consolidation_failure(
_error(message + " No operations were applied (batch is all-or-nothing).", usage=self._usage(target)))
_error(message + " No operations were applied (batch is all-or-nothing).", usage=self._usage(target),
**({"closest_entries": closest_entries} if closest_entries else {})))
def _mutate(self, target: str, mutate, *, skip_drift: bool = False) -> Dict[str, Any]:
"""Lock, re-read from disk, run ``mutate(entries, limit)`` -> ``(new_entries, message)``
@@ -308,11 +333,12 @@ class MemoryStore:
if content in entries:
return self._success_response(target, "Entry already exists (no duplicate added).")
if len(ENTRY_DELIMITER.join(entries + [content])) > limit:
new_total = len(ENTRY_DELIMITER.join(entries + [content]))
return self._failure_with_entries(target, (
f"Memory at {self._char_count(target):,}/{limit:,} chars. Adding this entry "
f"({len(content)} chars) would exceed the limit. Consolidate now: use 'replace' to merge "
f"overlapping entries into shorter ones or 'remove' stale or less important entries (see "
f"current_entries below), then retry this add — all in this turn."))
f"Memory at {self._char_count(target):,}/{limit:,} chars; adding this entry ({len(content)} chars) "
f"would exceed the limit by {new_total - limit:,} chars. Retry as ONE 'operations' batch that "
f"removes or shortens (replace) stale entries from current_entries below to free at least "
f"{new_total - limit:,} chars AND adds this entry — the limit is checked only on the batch result."))
return entries + [content], "Entry added."
# Append-only: skip the drift guard (appending never clobbers foreign
# content) but still refuse a failed read — add rewrites the WHOLE file.
@@ -352,9 +378,7 @@ class MemoryStore:
matches=[entries[i][:80] + ("..." if len(entries[i]) > 80 else "")
for i in _substring_matches(entries, old_text)])
if idx is None:
return self._consolidation_failure(_error(
f"No entry matched '{old_text}'. Check current_entries below and retry with the exact text "
f"of the entry you want to {verb}.", current_entries=entries))
return self._consolidation_failure(_no_match_error(entries, old_text, verb, current_entries=entries))
return idx
def resolve_entry(self, target: str, old_text: str, verb: str) -> Dict[str, Any]:
@@ -378,16 +402,17 @@ class MemoryStore:
new_total = len(ENTRY_DELIMITER.join(replaced))
if new_total > limit:
return self._failure_with_entries(target, (
f"Replacement would put memory at {new_total:,}/{limit:,} chars. Shorten the new content, "
f"or 'remove' other stale or less important entries to make room (see current_entries "
f"below), then retry — all in this turn."))
f"Replacement {_over_limit(new_total, limit)}. Shorten the new content, or retry as ONE "
f"'operations' batch with this replace plus removes/shortenings of other stale entries "
f"(see current_entries below) — the limit is checked only on the batch result."))
return replaced, "Entry replaced.", {"replaced_entry": entries[idx]}
return self._mutate(target, _apply)
@staticmethod
def _apply_batch_op(working: List[str], act: str, content: str, old_text: str,
pos: str, matched_entry: Optional[str] = None) -> Tuple[Optional[str], Optional[str]]:
"""Apply one batch op to *working*; return ``(error message, previous content)``.
"""Apply one batch op to *working*; return ``(error message, previous content)`` —
on a zero-match error the second value is that op's ``closest_entries`` instead.
Previous content is captured before each replace/remove, under the store lock.
It is published only after the entire batch has been validated and persisted.
A staged op's *matched_entry* selects exactly that entry, as in ``_locate``.
@@ -403,7 +428,8 @@ class MemoryStore:
if not old_text:
return f"{pos}: old_text is required.", None
if act == "replace" and not content:
return f"{pos}: content is required (use action='remove' to delete).", None
return (f"{pos}: content is required — set this operation object's own 'content' to the COMPLETE "
f"new entry (or use action='remove' to delete)."), None
if matched_entry is not None:
idx = _pinned_index(working, matched_entry)
if idx is None:
@@ -413,7 +439,8 @@ class MemoryStore:
if ambiguous:
return f"{pos}: '{old_text}' matched multiple distinct entries -- be more specific.", None
if idx is None:
return f"{pos}: no entry matched '{old_text}'.", None
miss = _no_match_error(working, old_text, act)
return f"{pos}: {miss['error']}", miss.get("closest_entries")
previous_content = working[idx]
working[idx:idx + 1] = [content] if act == "replace" else []
return None, previous_content
@@ -451,8 +478,8 @@ class MemoryStore:
working, act, (op.get("content") or op.get("new_text") or "").strip(),
(op.get("old_text") or "").strip(), f"Operation {i + 1} ({act or 'unknown'})",
op.get("matched_entry"))
if msg:
return self._batch_failure(target, msg)
if msg: # a failed op's second value is its closest_entries (or None)
return self._batch_failure(target, msg, closest_entries=previous_content)
matched.append(previous_content)
if entries and not working:
# #103419: a consolidation batch that removes the last entry would
@@ -467,9 +494,8 @@ class MemoryStore:
new_total = len(ENTRY_DELIMITER.join(working)) # budget check against the FINAL state only
if new_total > limit:
return self._batch_failure(target, (
f"After applying all {len(operations)} operations, memory would be at "
f"{new_total:,}/{limit:,} chars -- over the limit. Remove or shorten more "
f"entries in the same batch, then retry."))
f"Applying all {len(operations)} operations {_over_limit(new_total, limit)} more. Add "
f"removes or shorter replacements of other stale entries to the same batch, then retry."))
if not commit:
return {"success": True, "matched_entries": matched}
# op index -> full entry text its replace/remove selected (#117952), 1-based to
@@ -552,12 +578,13 @@ class MemoryStore:
raise RuntimeError(f"Failed to write memory file {path}: {e}")
def _detect_external_drift(self, target: str, raw: str) -> Optional[str]:
"""``.bak.<ts>`` snapshot path if *raw* shows external drift, else None. Signals:
round-trip mismatch, or one entry over the whole-file limit (no tool-written
entry can be — an external writer appended free-form text)."""
parsed = self._parse_entries(raw)
if not raw.strip() or (raw.strip() == ENTRY_DELIMITER.join(parsed)
and max(map(len, parsed), default=0) <= self._char_limit(target)):
"""``.bak.<ts>`` snapshot path if *raw* shows external drift, else None. The one
signal is an entry over the whole-file limit: no tool-written entry can be, so it is
free-form text appended without delimiters, and replacing that "entry" would discard
it. Layout differences (blank lines around ``§``, empty entries, padding) are NOT
drift: parsing only strips whitespace, so the flush loses nothing but whitespace, and
refusing there blocked every replace/remove on a hand-formatted file for good (#107270)."""
if max(map(len, self._parse_entries(raw)), default=0) <= self._char_limit(target):
return None
path = self._path_for(target)
bak_path = path.with_suffix(path.suffix + f".bak.{int(time.time())}")

11
website/docs/user-guide/features/memory.md
View File

@@ -171,17 +171,16 @@ When you try to add an entry that would exceed the limit, the tool returns an er
```json
{
"success": false,
"error": "Memory at 2,100/2,200 chars. Adding this entry (250 chars) would exceed the limit. Consolidate now: use 'replace' to merge overlapping entries into shorter ones or 'remove' stale or less important entries (see current_entries below), then retry this add — all in this turn.",
"error": "Memory at 2,100/2,200 chars; adding this entry (250 chars) would exceed the limit by 153 chars. Retry as ONE 'operations' batch that removes or shortens (replace) stale entries from current_entries below to free at least 153 chars AND adds this entry — the limit is checked only on the batch result.",
"current_entries": ["..."],
"usage": "2,100/2,200"
}
```
The agent should then:
1. Read the current entries (shown in the error response)
2. Identify entries that can be removed or consolidated
3. Use `replace` to merge related entries into shorter versions
4. Then `add` the new entry
The agent then reissues one `operations` batch that frees at least the stated number of
characters (removes, or `replace`s with shorter versions) and adds the new entry; the limit
is checked only on the batch's final result. A `replace`/`remove` whose `old_text` matches no
entry fails the same way, with the entries it most resembles under `closest_entries`.
**Best practice:** When memory is above 80% capacity (visible in the system prompt header), consolidate entries before adding new ones. For example, merge three separate "project uses X" entries into one comprehensive project description entry.