Merge pull request #52091 from NousResearch/salvage/42874-memory-drift-guard-add

fix(memory): skip drift guard for add (append-only) action (#42874)

tests/tools/test_memory_tool.py

@@ -604,16 +604,30 @@ class TestExternalDriftGuard:
         assert Path(bak).exists()
         assert "Vendor Master" in Path(bak).read_text()
 
-    def test_add_refuses_on_drift(self, store):
-        store.add("memory", "Existing.")
-        path = self._plant_drift(store)
-        original = path.read_text()
+    def test_add_succeeds_despite_drift(self, store):
+        """Add (append) should succeed even when on-disk content shows drift.
+
+        The drift guard protects replace/remove from clobbering un-roundtrippable
+        content, but add only appends — it never overwrites existing entries.
+        Issue #42874: prior-session add() writes shift the byte count, causing
+        the round-trip check to fire on subsequent adds in the same session.
+        """
+        store.add("memory", "Existing entry.")
+        # Plant a mild drift: append content that won't round-trip but stays
+        # under the char limit (500 chars in test fixture).
+        path = store._path_for("memory")
+        path.write_text(
+            path.read_text(encoding="utf-8") + "\nextra content no delimiter",
+            encoding="utf-8",
+        )
 
         result = store.add("memory", "New entry under drift.")
 
-        assert result["success"] is False
-        assert "drift_backup" in result
-        assert path.read_text() == original  # untouched
+        assert result["success"] is True
+        # The new entry is appended — existing drift content is preserved.
+        updated = path.read_text(encoding="utf-8")
+        assert "New entry under drift." in updated
+        assert "extra content no delimiter" in updated
 
     def test_remove_refuses_on_drift(self, store):
         store.add("memory", "Target entry to remove.")
@@ -668,12 +682,16 @@ class TestExternalDriftGuard:
         overwrite the first .bak. The current implementation accepts that
         — both files describe the same on-disk state — but pin the path
         format here so any future change has to think about it.
+
+        Note: add() no longer triggers drift detection (issue #42874) —
+        only replace/remove do.  Both r1 and r2 use replace/remove.
         """
         store.add("memory", "Initial.")
+        store.add("memory", "Second entry.")
         self._plant_drift(store)
 
         r1 = store.replace("memory", "Initial", "Replacement.")
-        r2 = store.add("memory", "Another.")
+        r2 = store.remove("memory", "Second entry")
         assert r1.get("drift_backup")
         assert r2.get("drift_backup")
         # Same epoch second is the expected collision case — both point

tools/memory_tool.py

@@ -248,7 +248,7 @@ class MemoryStore:
             return mem_dir / "USER.md"
         return mem_dir / "MEMORY.md"
 
-    def _reload_target(self, target: str) -> Optional[str]:
+    def _reload_target(self, target: str, *, skip_drift: bool = False) -> Optional[str]:
         """Re-read entries from disk into in-memory state.
 
         Called under file lock to get the latest state before mutating.
@@ -258,9 +258,13 @@ class MemoryStore:
         When drift is detected the caller must abort the mutation —
         flushing would discard the un-roundtrippable content.
         Returns None on clean reload.
+
+        When *skip_drift* is True the round-trip / entry-size check is
+        bypassed.  Used by the ``add`` action which appends without
+        rewriting, so existing content is never clobbered.
         """
         path = self._path_for(target)
-        bak = self._detect_external_drift(target)
+        bak = None if skip_drift else self._detect_external_drift(target)
         fresh = self._read_file(path)
         fresh = list(dict.fromkeys(fresh))  # deduplicate
         self._set_entries(target, fresh)
@@ -306,12 +310,12 @@ class MemoryStore:
 
         with self._file_lock(self._path_for(target)):
             # Re-read from disk under lock to pick up writes from other sessions.
-            # If external drift was detected, the file was backed up to .bak.<ts>
-            # — refuse the mutation so we don't clobber the un-roundtrippable
-            # content the patch tool / shell append / sister session wrote.
-            bak = self._reload_target(target)
-            if bak:
-                return _drift_error(self._path_for(target), bak)
+            # For add (append-only), we skip the drift guard — appending never
+            # clobbers existing content, so round-trip mismatches from prior
+            # tool-written entries in the same session are harmless.  The drift
+            # guard remains active for replace/remove where full-file rewrite
+            # would discard un-roundtrippable content (issue #26045).
+            self._reload_target(target, skip_drift=True)
 
             entries = self._entries_for(target)
             limit = self._char_limit(target)