fix(memory): tolerate non-UTF-8 bytes in USER.md/MEMORY.md reads

[Bartok9]

Closes #53833.

Root Cause

Symptom — When USER.md or MEMORY.md contains a byte outside valid UTF-8 (e.g. a smart-quote/dash saved under a cp1252 mismatch), every subsequent memory save crashes with UnicodeDecodeError: 'utf-8' codec can't decode byte 0xd1 ... invalid continuation byte, permanently disabling memory writes for that profile (reporter also saw cascading send-pipeline failures in the same broken session).

Root cause — MemoryStore._read_file() (and _detect_external_drift()) read the file with path.read_text(encoding="utf-8") and only catch (OSError, IOError). A UnicodeDecodeError is a ValueError, not an OSError, so it escapes the guard and propagates up through _reload_target → add → memory_tool.

Evidence — tools/memory_tool.py:639 and :683. New regression test tests/tools/test_memory_tool.py::TestNonUtf8Bytes::test_read_file_replaces_invalid_bytes reproduces the exact traceback on current main:

E   UnicodeDecodeError: 'utf-8' codec can't decode byte 0xd1 in position 31: invalid continuation byte
<frozen codecs>:322: UnicodeDecodeError
1 failed

Fix + why this level — Add errors="replace" to both reads, matching the issue's suggested fix. This is the correct layer: the file is already on disk with bad bytes, so the read must be made resilient (catching the exception elsewhere would lose the content entirely and still drop the entry). errors="replace" substitutes the standard U+FFFD replacement char and preserves all surrounding valid content, so memory keeps working.

Scope / risk — Touches only the two read paths in memory_tool.py. Valid UTF-8 files are byte-identical (no behavior change); only previously-crashing files now read with replacement chars. Writes already go through atomic rename and re-serialize as UTF-8, so a load→save cycle naturally heals the bad bytes.

Verification

• python -m pytest tests/tools/test_memory_tool.py — 78 passed
• New TestNonUtf8Bytes (2 tests): test_read_file_replaces_invalid_bytes, test_load_from_disk_survives_invalid_bytes. Both FAIL without the fix (UnicodeDecodeError) and pass with it.

Real behavior proof

After fix, running the new regression tests on this branch:

tests/tools/test_memory_tool.py::TestNonUtf8Bytes::test_read_file_replaces_invalid_bytes PASSED
tests/tools/test_memory_tool.py::TestNonUtf8Bytes::test_load_from_disk_survives_invalid_bytes PASSED
2 passed in 0.10s

[alt-glitch]

This was generated by AI during triage.

Related: #53833 (the issue this fixes), #10888 and #18157 (open PRs in the same memory non-UTF-8/encoding family). Same robustness goal via overlapping mechanisms — not a clean duplicate; flagging the cluster for maintainer consolidation.

[tonydwb]

Code Review Summary

Verdict: Approved

Fixes UnicodeDecodeError when reading USER.md/MEMORY.md files with stray non-UTF-8 bytes (e.g. smart quotes saved under cp1252 mismatch). Changes read_text(encoding='utf-8') to read_text(encoding='utf-8', errors='replace') in both _read_file and _detect_external_drift. The errors='replace' mode substitutes replacement characters for invalid bytes, preventing the error from wedging every future memory save. Clean single-line fix with two regression tests.

---

Reviewed by Hermes Agent

[Bartok9]

Thanks for mapping the cluster. Agreed this isn't a clean duplicate — #53833 is the specific issue this fixes, while #10888 and #18157 tackle the broader encoding family via different mechanisms. This PR is intentionally narrow (USER.md/MEMORY.md reads only). Happy to coordinate consolidation if a maintainer wants the wider robustness pass unified, but this one is safe to land standalone.

[teknium1]

Thanks for the narrowly scoped robustness fix. Live main still strictly decodes both relevant memory reads at tools/memory_tool.py:692 and tools/memory_tool.py:732, so the premise remains valid.

Problems

• tests/tools/test_memory_tool.py:796-812 verifies loading but does not exercise the changed drift-check route used by replace/remove. It also does not assert the promised replacement-character behavior; assert len(entries) == 2 alone would also pass if bad bytes were silently ignored.

Suggested changes

• Assert that the invalid byte decodes to \uFFFD.
• Add a malformed-file mutation test that reaches _reload_target / _detect_external_drift and verifies a replace or remove completes successfully.

Automated hermes-sweeper review.

[teknium1]

This only proves decoding did not raise and the delimiter still split the file; errors="ignore" would also satisfy it. Please assert that the second entry contains \uFFFD so the regression protects the requested replacement semantics.

[Bartok9]

Thanks @teknium1 — both review items addressed on the latest push (rebased onto current main):

1. U+FFFD assertion — test_read_file_replaces_invalid_bytes and test_load_from_disk_survives_invalid_bytes now require "\uFFFD" in the decoded entry (so errors="ignore" would fail).
2. Mutation path — added test_replace_via_reload_target_survives_invalid_bytes and test_remove_via_reload_target_survives_invalid_bytes, which hit _reload_target → _detect_external_drift + _read_file with a malformed on-disk file and assert replace/remove complete successfully.

Local: pytest tests/tools/test_memory_tool.py::TestNonUtf8Bytes → 4 passed.

[teknium1]

Closing as redundant with credit: current main's memory_tool._read_raw_checked already catches UnicodeDecodeError with a stronger contract than replace-and-continue — a failed read returns read_ok=False and ABORTS the write path, so a mis-decoded view can never persist over (and wipe) the on-disk memory (the #26045 class). Your diagnosis of the crash was correct; the shipped design just resolves it at a different layer. The broader bare-file-I/O class was retired in PR #71078. Thanks for the report and fix.