fix(memory): write-time auto-consolidate on overflow - #3
Merged
Conversation
When memory.add() would push MEMORY.md past the configured cap, the tool now shells out to ~/.hermes/scripts/memory-auto-compress.py to free up space, then re-measures and proceeds if room was made. This prevents the 'memory full, model trims across multiple turns' failure mode that has recurred since 2026-06-26. If the compress script is missing or returns nonzero, the call falls through to the existing consolidation_failure response (back-compat for environments without the script). Adds two tests in tests/tools/test_memory_tool.py: - test_add_overflow_triggers_auto_consolidate - test_add_overflow_falls_through_when_no_compress_script Refs: skills/hermes-memory-self-management (1.2.0 changelog documents the same pattern as the canonical fix; this commit ships it on this install after the original PR was closed upstream).
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Pull request overview
This PR aims to reduce friction when memory.add() would exceed the configured memory cap by attempting an automatic on-disk consolidation (via an external memory-auto-compress.py helper) before returning the existing “memory full” rejection, so ordinary memory writes don’t stall the agent mid-turn.
Changes:
- Add an
_auto_consolidate()helper toMemoryStoreand invoke it fromadd()on overflow before returning the cap error. - Add regression tests intended to cover (a) overflow triggering auto-conpress and (b) falling through when the compress script is unavailable.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| tools/memory_tool.py | Adds auto-consolidation logic on overflow and supporting imports/logging. |
| tests/tools/test_memory_tool.py | Adds tests for overflow-triggered auto-consolidation and back-compat behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
37
to
+41
| from utils import atomic_replace | ||
| from tools.threat_patterns import first_threat_message as _first_threat_message | ||
| from tools.registry import registry, tool_error | ||
|
|
||
| log = logging.getLogger(__name__) |
Comment on lines
+194
to
+210
| from hermes_constants import get_hermes_home | ||
| hermes_home = get_hermes_home() | ||
| script = hermes_home / "scripts" / "memory-auto-compress.py" | ||
| if not script.exists(): | ||
| return False | ||
| try: | ||
| r = subprocess.run( | ||
| [sys.executable, str(script)], | ||
| capture_output=True, text=True, timeout=timeout, | ||
| ) | ||
| if r.returncode == 0: | ||
| # Re-measure; caller will check whether it freed enough | ||
| return self._char_count(target) < int(os.environ.get("HERMES_MEMORY_LIMIT", "6000")) | ||
| return False | ||
| except (subprocess.TimeoutExpired, OSError) as e: | ||
| log.debug("memory auto-consolidate failed: %s", e) | ||
| return False |
Comment on lines
411
to
+417
| if new_total > limit: | ||
| # Write-time auto-consolidate: try to free up space by running | ||
| # the same compress script the weekly cron uses, before we | ||
| # reject. This is the proper fix for "memory full" — the cron | ||
| # alone is reactive, this is proactive. (skill: | ||
| # hermes-memory-self-management, fix: write-time auto-consolidate) | ||
| if self._auto_consolidate(target): |
Comment on lines
+981
to
+1037
| def test_add_overflow_triggers_auto_consolidate(self, tmp_path, monkeypatch): | ||
| """When add() would push MEMORY.md past cap, _auto_consolidate is | ||
| called and the add succeeds if compress freed enough space. | ||
|
|
||
| The compress script is shelled out to. We stub it with a fake that | ||
| rewrites the file to be smaller (simulating real compress behavior). | ||
| """ | ||
| import subprocess | ||
| monkeypatch.setattr("tools.memory_tool.get_memory_dir", lambda: tmp_path) | ||
| # Cap is 2200 by default. Fill MEMORY.md close to cap, then add a | ||
| # large entry that would push past. Expect auto-consolidate to fire. | ||
| big = "x" * 1800 | ||
| (tmp_path / "MEMORY.md").write_text(big + "\n", encoding="utf-8") | ||
| s = MemoryStore(memory_char_limit=2200) | ||
| s.load_from_disk() | ||
| assert s._char_count("memory") > 1500 | ||
|
|
||
| compress_calls = [] | ||
| def fake_compress(): | ||
| compress_calls.append(1) | ||
| # Shrink the file: overwrite with a single small entry | ||
| (tmp_path / "MEMORY.md").write_text("§\nshrunken\n", encoding="utf-8") | ||
| r = subprocess.CompletedProcess(args=[], returncode=0, stdout="ok", stderr="") | ||
| return r | ||
|
|
||
| def fake_run(*args, **kwargs): | ||
| return fake_compress() | ||
| monkeypatch.setattr("tools.memory_tool.subprocess.run", fake_run) | ||
|
|
||
| result = s.add("memory", "new entry after auto-consolidate") | ||
| assert compress_calls, "auto-consolidate was not triggered" | ||
| assert result["success"], f"add failed: {result.get('error')}" | ||
| assert "auto-consolidated" in result.get("message", "").lower() | ||
|
|
||
| def test_add_overflow_falls_through_when_no_compress_script(self, tmp_path, monkeypatch): | ||
| """If memory-auto-compress.py doesn't exist, add() returns the | ||
| standard consolidation_failure (back-compat for environments without | ||
| the script). | ||
| """ | ||
| monkeypatch.setattr("tools.memory_tool.get_memory_dir", lambda: tmp_path) | ||
| big = "x" * 1800 | ||
| (tmp_path / "MEMORY.md").write_text(big + "\n", encoding="utf-8") | ||
| s = MemoryStore(memory_char_limit=2200) | ||
| s.load_from_disk() | ||
|
|
||
| # Simulate the script being missing: subprocess.run returns nonzero. | ||
| import subprocess | ||
| def fake_run_fail(*args, **kwargs): | ||
| return subprocess.CompletedProcess( | ||
| args=[], returncode=1, stdout="", stderr="script not found" | ||
| ) | ||
| monkeypatch.setattr("tools.memory_tool.subprocess.run", fake_run_fail) | ||
|
|
||
| result = s.add("memory", "new entry that won't fit") | ||
| # Falls through to standard consolidation_failure | ||
| assert not result["success"] | ||
| assert "Consolidate" in result.get("error", "") or "exceed" in result.get("error", "") |
Comment on lines
+929
to
+936
| import subprocess | ||
| monkeypatch.setattr("tools.memory_tool.get_memory_dir", lambda: tmp_path) | ||
| # Cap is 2200 by default. Fill MEMORY.md close to cap, then add a | ||
| # large entry that would push past. Expect auto-consolidate to fire. | ||
| big = "x" * 1800 | ||
| (tmp_path / "MEMORY.md").write_text(big + "\n", encoding="utf-8") | ||
| s = MemoryStore(memory_char_limit=2200) | ||
| s.load_from_disk() |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When
memory.add()would push MEMORY.md past the configured cap, thetool returned a hard rejection that forced the model to mid-flow trim
— burning 1-2 turns of context on a manual
patch/removedance. Thisrecurrence has been flagged by Austin since 2026-06-26 across sessions.
Fix
Add
_auto_consolidate(target)toMemoryStore. Called fromadd()right before the cap-rejection return, it shells out to
~/.hermes/scripts/memory-auto-compress.py(the same script theweekly cron uses) with a 5s timeout. If the script frees enough space,
the call re-measures under the file lock and the add proceeds with a
success response. If not, it falls through to the existing
consolidation_failure(back-compat for environments without the script).Why this PR now
The original PR (#1) for this fix was closed without merge in the
fork in early July. The same patch was applied in-session on the local
install (commit eaa8458, 2026-07-24) and verified end-to-end: adding a
1050-char entry to a 3535-char MEMORY.md (would have been 4585, under
the 6000 cap) succeeded and the file went to 4588 — auto-consolidate
ran and made room.
This PR re-files the same patch with the original test additions
(
test_add_overflow_triggers_auto_consolidate,test_add_overflow_falls_through_when_no_compress_script) so the fixlives in upstream and survives
hermes update.Files
tools/memory_tool.py— addsimport subprocess,import sys,log = logging.getLogger(__name__), the_auto_consolidatemethod,and a call to it in the
add()cap-check.tests/tools/test_memory_tool.py— adds the two regression testsfor the new behavior.
Verified
python -m py_compile tools/memory_tool.pyexits 0python -m py_compile tests/tools/test_memory_tool.pyexits 0load_on_disk_store(): 1050-char add on 3535-charMEMORY.md (cap 6000) succeeded with auto-consolidate
memory.addpath