From 2c1efa29ec5f4fb91a85c0b36ae0920dddcf5458 Mon Sep 17 00:00:00 2001 From: Luan Vu Date: Thu, 19 Mar 2026 17:42:03 +0700 Subject: [PATCH] fix(memory): prevent silent data loss on memory file overwrites (#279) * fix(memory): auto-backup + overwrite warning for memory files When an agent writes to a memory file that already has content: 1. Old content is backed up to memory/.prev/{filename} (best-effort) 2. Write succeeds normally (no blocking) 3. Tool result includes warning with previous content (truncated to 4000 chars) so the agent can notice and merge if needed Backup files are hidden from list_files and not indexed/embedded. No database migration needed - uses existing PutDocument. * fix(memory): support append mode + fix UTF-8 truncation in overwrite warning - Add appendMode parameter to MemoryInterceptor.WriteFile() so write_file(append=true) actually appends to memory files instead of silently replacing content. Uses "\n\n---\n\n" separator. - Fix UTF-8 truncation in overwrite warning: use []rune instead of byte slicing to avoid cutting multi-byte characters. - Fix char count in warning message: use len([]rune()) not len(). - Remove .prev/ backup mechanism (prevBackupPath, isPrevPath, backup writes, ListFiles filter) in favor of simpler approach: append mode prevents data loss, warning enables self-correction. --------- Co-authored-by: Luvu182 <208665161+Luvu182@users.noreply.github.com> Co-authored-by: viettranx --- internal/tools/edit.go | 2 +- internal/tools/filesystem_write.go | 14 +++++++++++- internal/tools/memory_interceptor.go | 34 ++++++++++++++++++++-------- 3 files changed, 39 insertions(+), 11 deletions(-) diff --git a/internal/tools/edit.go b/internal/tools/edit.go index 9cd0414e..aa2fc76a 100644 --- a/internal/tools/edit.go +++ b/internal/tools/edit.go @@ -137,7 +137,7 @@ func (t *EditTool) Execute(ctx context.Context, args map[string]any) *Result { if result != nil { return result } - mwr, err := t.memIntc.WriteFile(ctx, path, newContent) + mwr, err := t.memIntc.WriteFile(ctx, path, newContent, false) if err != nil { return ErrorResult(fmt.Sprintf("failed to write memory file: %v", err)) } diff --git a/internal/tools/filesystem_write.go b/internal/tools/filesystem_write.go index 0f80df9b..17bb1d62 100644 --- a/internal/tools/filesystem_write.go +++ b/internal/tools/filesystem_write.go @@ -121,7 +121,7 @@ func (t *WriteFileTool) Execute(ctx context.Context, args map[string]any) *Resul // Virtual FS: route memory files to DB if t.memIntc != nil { - if mwr, err := t.memIntc.WriteFile(ctx, path, content); mwr.Handled { + if mwr, err := t.memIntc.WriteFile(ctx, path, content, appendMode); mwr.Handled { if err != nil { return ErrorResult(fmt.Sprintf("failed to write memory file: %v", err)) } @@ -129,6 +129,18 @@ func (t *WriteFileTool) Execute(ctx context.Context, args map[string]any) *Resul if mwr.KGTriggered { msg += "\n\n[Knowledge graph extraction triggered in background. The knowledge system may take a moment to fully update with new entities and relationships.]" } + if mwr.PreviousContent != "" { + prev := mwr.PreviousContent + prevRunes := []rune(prev) + if len(prevRunes) > 4000 { + prev = string(prevRunes[:4000]) + "\n... (truncated)" + } + msg += fmt.Sprintf("\n\n⚠️ WARNING: This file had existing content (%d chars) that was replaced. "+ + "If the old content below contains information not present in your new version, "+ + "please re-write the file to merge both.\n\n"+ + "--- PREVIOUS CONTENT ---\n%s\n--- END PREVIOUS CONTENT ---", + len([]rune(mwr.PreviousContent)), prev) + } return SilentResult(msg) } } diff --git a/internal/tools/memory_interceptor.go b/internal/tools/memory_interceptor.go index f7bc2d3f..e9f78e1e 100644 --- a/internal/tools/memory_interceptor.go +++ b/internal/tools/memory_interceptor.go @@ -127,15 +127,16 @@ func (m *MemoryInterceptor) ReadFile(ctx context.Context, path string) (string, // MemoryWriteResult holds the outcome of a memory write operation. type MemoryWriteResult struct { - Handled bool - KGTriggered bool + Handled bool + KGTriggered bool + PreviousContent string // non-empty if an existing document was overwritten (non-append) } // WriteFile attempts to write a memory file to the DB (+ re-index chunks for .md files). -// Non-.md files are stored but NOT indexed/chunked/embedded, -// matching TS behavior where only .md files are indexed. -// Returns MemoryWriteResult with Handled=true if this was a memory path, KGTriggered=true if KG extraction was started. -func (m *MemoryInterceptor) WriteFile(ctx context.Context, path, content string) (MemoryWriteResult, error) { +// When appendMode is true, new content is appended to the existing document with a separator. +// When appendMode is false and an existing document is overwritten with different content, +// PreviousContent is populated in the result to allow callers to warn the agent. +func (m *MemoryInterceptor) WriteFile(ctx context.Context, path, content string, appendMode bool) (MemoryWriteResult, error) { ws := effectiveWorkspace(ctx, m.workspace) if !isMemoryPath(path, ws) { return MemoryWriteResult{}, nil @@ -152,13 +153,28 @@ func (m *MemoryInterceptor) WriteFile(ctx context.Context, path, content string) userID := store.MemoryUserID(ctx) agentStr := agentID.String() + var previousContent string + + if appendMode { + // Append: read existing content and merge with separator. + existing, err := m.memStore.GetDocument(ctx, agentStr, userID, relPath) + if err == nil && existing != "" { + content = existing + "\n\n---\n\n" + content + } + } else { + // Replace: capture previous content for overwrite warning. + oldContent, err := m.memStore.GetDocument(ctx, agentStr, userID, relPath) + if err == nil && oldContent != "" && oldContent != content { + previousContent = oldContent + } + } + // Write document to DB if err := m.memStore.PutDocument(ctx, agentStr, userID, relPath, content); err != nil { return MemoryWriteResult{Handled: true}, err } - // Only index .md files (chunk + embed). Non-.md files (JSON, etc.) are stored - // as key-value documents but not searchable via memory_search. + // Only index .md files (chunk + embed). if strings.HasSuffix(relPath, ".md") { if err := m.memStore.IndexDocument(ctx, agentStr, userID, relPath); err != nil { slog.Warn("memory interceptor: index failed after write", "path", path, "error", err) @@ -173,7 +189,7 @@ func (m *MemoryInterceptor) WriteFile(ctx context.Context, path, content string) kgTriggered = true } - return MemoryWriteResult{Handled: true, KGTriggered: kgTriggered}, nil + return MemoryWriteResult{Handled: true, KGTriggered: kgTriggered, PreviousContent: previousContent}, nil } // ListFiles lists memory documents from the DB when path is the memory directory.