`_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.
