fix(memory): refuse mutations when on-disk file changed since last read (#26045)

[bradhallett]

Summary

Follow-up to #26045 (commit 6855d17). The drift guard detects structurally un-roundtrippable content, but jrhouston-trilogy recommended an additional defense: refuse mutations when the on-disk file has changed since last read — the same contract the patch tool already enforces.

This alone would have prevented the original ~8KB data-loss incident.

Changes

• tools/memory_tool.py: Added SHA-256 content-hash snapshot at lock acquisition. Before save_to_disk(), re-reads the file and refuses if the hash differs. Applied to all three mutation paths (add, replace, remove).
• tests/tools/test_memory_tool.py: 6 new tests in TestOnDiskChangeGuard

Relationship to existing drift guard

Both defenses are active and independent:

1. Drift guard fires first — detects structurally un-roundtrippable content (missing delimiters, oversized entries)
2. On-disk guard fires second — detects any content change between read and write, even if structurally valid

Test plan

pytest tests/tools/test_memory_tool.py -v   # 46 passed (40 existing + 6 new)

Refs #26045

[jsboige]

Reviewed this carefully. The on-disk change guard is well-designed:

What it does: SHA256 snapshot of file content before lock acquisition, then verification before write in add/replace/remove. Refuses mutation if content changed between read and write — prevents data loss from concurrent sessions (#26045).

Strengths:

• Static methods (_snapshot_on_disk, _check_on_disk_unchanged) — clean separation, testable
• Guard fires AFTER drift check — correct ordering (drift is structural, on-disk is temporal)
• 7 tests covering all 3 mutation paths + edge cases (new file, drift vs on-disk independence)
• Error messages reference issue #26045 and instruct user to re-read + retry

One observation (non-blocking): The pre_hash is taken just before _reload_target() which re-reads from disk. If the file changes between _snapshot_on_disk() and _reload_target(), the drift guard would likely catch it (unparseable content → backup). If it changes between _reload_target() and the check, the on-disk guard catches it. This covers the full window — good.

LGTM.

[teknium1]

Thanks for pursuing the concurrent-write protection from #26045. The underlying race remains on current main: replace reloads at tools/memory_tool.py:403 and saves at :453 without an on-disk version check.

Problems

• The proposed pre_hash is captured before _reload_target (tools/memory_tool.py:294 in this diff). A valid write between those reads is incorporated by the reload but still causes a refusal. Snapshot after the authoritative reload instead.
• The add path appends to live entries before checking (tools/memory_tool.py:328), so a refused add leaves in-memory state changed even though disk was not written.
• Current main added apply_batch in 38c8a9c10; it also reloads then saves (tools/memory_tool.py:524, :600) and needs the same protection.
• The new tests mock the check helper, rather than modifying the backing file during the real reload-to-write window.

Suggested changes

• Centralize a post-reload snapshot plus immediately-pre-write validation for every persistence path, including apply_batch.
• Add real temp-file mutation tests and assert both disk and live state remain unchanged on refusal.

This is an automated hermes-sweeper review.

[teknium1]

_reload_target() reads the file again immediately after this snapshot. If a valid external write lands between these reads, the reload already incorporates it, but the later check still rejects the mutation. Capture the snapshot after the authoritative reload so the guard covers only changes after the state used for this mutation.

[teknium1]

This refusal happens after entries.append(content), so a failed add leaves memory_entries/user_entries changed in memory although disk was not written. Validate before mutating live entries, or restore the previous list before returning the error.