Fleet telemetry shows built-in memory writes failing 34-47% and automatic
compression failing ~15%, and several merged fixes each target one cause
(#133038 for memory, #133054/#133040/#133051/#133032 for compression). Both
metrics only carried outcome, so the fleet could not tell which cause a fix
moved.
- hermes.memory.op.count and hermes.compression.count gain one closed-enum
dimension, failure_class; `none` on success.
- Memory: every refusal/failure return in tools/memory_tool.py and
tools/memory_tool_store.py names its class (no_match, ambiguous,
over_budget, would_empty, drift, read_failed, retry_cap, scan_blocked,
missing_content, missing_old_text, invalid_args, stale_entry, disabled,
staged, gate_refused) through a context variable; the result the model
sees is unchanged. Plugin providers report provider_error / exception.
- Compression: the attempt log's own failure_class, collapsed onto the
compressor's closed names (exception:<Type> / rollback:<Type> keep only the
prefix, anything else reads other, a classless failure unknown). Skipped
rows carry the skip reason, and the skip set is now defined once in the
contract.
- Package schema v4 / event schema v4; v3 field sets of both metrics stay
valid so pending rows drain. Doc table rows and the consent copy (CLI
setup and every Desktop locale's collectedActivity) name the new reason.
The typography-folded fallback in _substring_matches rejected a needle only when
it was quotes/dashes at the ends (needle.strip("'-")), so old_text="\u201c \u2014"
(curly quote, space, em dash) folded to "' -" and matched any entry containing
`" -`, deleting it via single or batch remove. The fallback now requires at least
one alphanumeric character; exact-entry and byte-substring priority are unchanged.
Tests: the PR's four new tests are folded into two invariants (layout/typography
match + punctuation-only rejection; self-correcting failure copy + #90468 alias).
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.
Background-review (and human /memory) replaces re-emit entry text with
different quote characters (straight ' vs stored ") or re-wrapped lines, so
byte-exact old_text substring matching wrongly reported 'No entry matched'
and the staged consolidation write failed to apply. Fall back to a
quote/whitespace-normalized match only when no exact match exists, preserving
today's unambiguous behavior. Adds regression tests for the quote-style
replace and for unrelated old_text still being rejected.
`_batch` kept three accumulators holding the same selected entry; keep only
the per-op `matched` list and build `replaced_entries`/`removed_entries` from it
on commit. The background delete gate now asks `destructive_ops` instead of
re-spelling the predicate, `destructive_ops` tolerates a None op like the rest
of the batch path, and the pinned lookup no longer threads a dead `ambiguous`.
resolve_batch_entries copy-pasted apply_batch's op loop and had already
drifted from it: no `op or {}` normalisation, no pre-disk content scan, no
empty-store or budget check. A background-review batch carrying an
injection payload was therefore staged (and echoed in /memory pending) and
only rejected at approve time. One private _batch(commit=...) now backs
both: apply_batch persists, resolve_batch_entries returns the per-op
matched entries from the same validated walk, so pin time fails exactly
where the direct batch would.
Also fold the twice-written "pinned entry -> index or stale" lookup into
_pinned_index, and state in _mutate's docstring that any dict a closure
returns is passed through unpersisted (the read-only resolve closures
rely on that, not only error dicts).
A staged replace/remove recorded only its old_text search string, and
/memory approve re-ran that search against the file as it was at
approval time. The background review stages every replace/remove it
wants (#105921), so when the live agent updated the targeted entry in
place and the new text still contained old_text, approving deleted or
overwrote the newer entry the approver never saw, and the output said
only "Approved 1 memory write(s)."
- Both staging gates resolve each replace/remove, under the store lock,
to the full entry it matches and record it as matched_entry. No match
or an ambiguous match is refused at staging, as the direct write is.
- Approval matches that entry exactly. If it changed since staging, the
write is refused and the pending record is kept, as #96059 does for
skills.
- /memory approve lists removed entries next to overwritten ones, so a
record staged before this change (no matched_entry, still replayed by
old_text) never removes an entry silently.
- /memory pending shows the full entry each staged replace/remove
targets, not just its search string.
(cherry picked from commit 28700fa750c9321ded48be77a76531b5809f5fea)
A Journey memory edit or delete (Desktop panel, `hermes journey`, TUI
`learning.*`) rewrote the whole MEMORY.md / USER.md from an unlocked
snapshot via `_read_file` + `_write_file`: a memory the agent stored in
between was dropped, and hand-edited content that does not round-trip
through the § parser was reformatted with no .bak.
Both mutations now go through `MemoryStore._mutate` — the memory tool's
cross-process lock, re-read under lock and drift guard. The node id is
resolved to its entry text inside the lock and matched by exact text
against the re-read entries; a vanished target, drift or an unreadable
file is refused with the store's own message instead of written over.
Router/CLI/TUI response shapes are unchanged.
Fixes#119668
Quality fold on the #117952 salvage stack: replaced_entries keys now match
the 1-based "Operation N" numbering the model sees for failed ops in the
same batch response (0-based dict keys JSON-serialialize to "0" and would
misalign with "Operation 1"); _apply_batch_op docstring states the actual
None/None-success semantics; the missing-old_text replace hint is hoisted
out of the f-string concat.
90/90 across the memory, schema, import-fallback, write-approval,
background-review-scope and honho-write suites.
Reuse fold on the #117952 salvage stack: _mutate's optional third value was
plumbed through a (field-name, lambda-extractor) tuple + extra kwarg + two
module constants. _error already had the convention this needed — caller
supplies a dict, the response merges it. _apply closures now return the
payload dict directly ("replaced_entry"/"replaced_entries") and
_success_response takes **extra, dropping the constants, the extractor
lambdas and ~8 lines of machinery. Add/remove-only batches no longer carry
a noise field (the closure omits the key when nothing was replaced).
79/79 across memory + schema + write-approval suites; E2E re-run on the
final shape (single/batch/replay visibility present, no noise on
remove-only batches); mutation check re-proven (dropping the merge turns
test_replace_whole_entry_contract red).
Review fold on the #117952 salvage stack:
- test_replace_same_across_single_batch_and_approval_replay built three
stores over ONE shared store dir, so the batch/replay stages appended onto
the single-stage result and read its leftover entry back — the cross-surface
equality passed without exercising the batch or replay surfaces. Each
surface now gets its own dir; the batch visibility assertion moved into
test_replace_whole_entry_contract where it binds the batch surface directly.
- _BATCH_REPLACED_ENTRIES now maps an empty dict to None so add/remove-only
batches don't carry a "replaced_entries": {} noise field in every result.
Mutation re-check: reverting the batch visibility field turns
test_replace_whole_entry_contract red; 61/61 suite green after the fold.
A partial-entry replace (old_text = a span inside an entry, content = its
replacement) silently overwrote the WHOLE entry: every clause outside the
match was destroyed with success=true, most damagingly through the
write-approval replay path (/memory approve -> apply_memory_pending ->
apply_batch) that exists precisely so background agents can stage writes for
human review (#117952, dup of #59184).
The cluster's repeated fix attempt (#66321, #61357, four closed predecessors)
flipped replace to naive span-splicing (entry.replace(old_text, new, 1)).
That corrupts the canonical identifier-style call (replace(old_text="3.11",
content="Python 3.12 project") -> "Python Python 3.12 project project"),
breaks the background-review refine consumer, and split-brains external
memory mirrors, which receive the raw op content, not a merged entry.
The defect is the CONTRACT, not the mechanism: the schema invited patch-style
calls ("mirrors old_text (patch-tool shape)") while the store commits
whole entries. This pins whole-entry semantics end to end and makes the
overwrite visible instead of silent:
- schema + recovery error text + memory_tool docstring now say content is the
COMPLETE new entry and old_text only locates it (website docs updated in the
same PR)
- a successful replace surfaces replaced_entry / replaced_entries (full text
that was overwritten) on single, batch, and approval-replay paths, so the
caller can re-add lost clauses instead of discovering them gone later
- _mutate gains an optional extra payload field; _apply_batch_op returns the
replaced entry text alongside the error
Live repro on origin/main dec236b21 before: approve-apply of a staged
partial-entry replace truncated a 3-clause entry to the replacement span
alone. After: same final entry (whole-entry contract, unchanged semantics)
plus replaced_entries in the response. Grafted naive span-splice on main:
2 existing tests red (test_replace_entry,
test_attended_review_keeps_full_operation_set) — the regression this PR
avoids. Mutation checks: reverting the visibility fields or the exact-match
priority turns the new invariant tests red.
A short entry whose full text is contained inside a longer sibling entry
was unaddressable: remove('test') against entries ['test', '...tests
pass...'] hit _find_unique_match's substring scan, reported 'Multiple
entries matched', and refused — the entry could never be targeted.
apply_batch hit the same matcher and aborted the whole operation.
_find_unique_match() now prefers whole-entry EXACT matches
(old_text == entry) and only falls back to substring matches when no
entry equals old_text. Both call sites (_edit for single replace/remove,
_apply_batch_op for batch) share the matcher, so the whole bug class is
fixed at one seam. Substring partial matching is unchanged.
Reproduced red on current main (2 invariant tests), 53/53 green after
the fix; 8 sibling memory suites green via scripts/run_tests.sh.
A failed all-or-nothing consolidation must not echo the whole
store back into the turn. Single replace/remove still return
the entry list so the model can retry with exact text.
Background writers that still carry a tombstoned profile as their Hermes
home (reasoning-caps warm thread, models cache, models.dev ETag, gateway
lifecycle ledger, MCP OAuth tokens, memory store) re-created
profiles/<name>/ with a bare mkdir right before an atomic write. Route the
parent-dir creation through mkdir_under_hermes_home so a deleted named
profile raises FileNotFoundError and stays gone, matching the tombstone
contract already enforced for logging and state.
The cap only fires on add/replace, so an externally written over-budget
file rode silently in the system prompt while every later add was refused
with no visible cause. Warn at load; entries stay loaded (never truncate
a user's memories).
Salvage of #10886 (original hunk targeted memory_tool.py before the
store split); authored by @easyvibecoding.
Refs #10877
Review follow-up on the salvaged #103421 guard: the USER.md/MEMORY.md ternary
was a third copy (also _path_for and memory_tool._memory_target_error); use the
path's name. "profile" → "store" in the error since MEMORY.md is not a profile.
apply_batch() committed an empty USER.md/MEMORY.md as a normal
successful write when a consolidation batch removed the last entry.
Refuse all-or-nothing with live entries so background consolidation
keeps at least one entry; a deliberate wipe stays a manual file edit.
For each issue anchor present in BASE 63279301bc non-test .py and absent on HEAD, the BASE comment/docstring block was re-attached at the HEAD location of the code it explained (matched by the distinctive code line / enclosing def). Sentences already covered by an existing HEAD comment were deduped; the issue number always survives. Insert-only: no code lines changed.
memory_tool_store: _error() helper for every failure dict, reload folded into
_mutate, load_from_disk loops over targets with an inline snapshot sanitizer,
_locate inlined into _edit, batch-op splice, drift check compacted; on-disk
format and every result string unchanged (golden corpus).
memory_tool: single _apply_write_gate handles op + batch, _batch_op_line helper,
gate/validation returns folded, on-disk store built directly.
session_search_tool: _get_session_meta unifies 5 get_session lookups, rebuild
note folded into _discover_payload via _ok, lineage dedupe inlined into
_discover, scroll rebind inlined, title-match shaping via one closure, browse
comprehension, unreachable scroll guard dropped, single hermes_state import.
microsoft_graph_client/auth: signatures hugged, bodiless-response handling
inlined, header dict built in one expression, dead from_env removed.
process_registry_notifications: _preamble merges header/task-source/role lines,
_notice_lines shared, table-driven completion status, comprehension headers.
Schemas byte-identical (SCHEMA-OK); golden corpus old-vs-new identical.
