fix(memory): refuse deletion after read failure

Author: gnanam1990

internal/tools/memory.go

@@ -270,8 +270,14 @@ func (tool memoryForgetTool) Run(_ context.Context, args map[string]any) Result
 	// missing note is not an error at the store layer — but saying "Forgot" for a
 	// note that never existed tells a model which misspelled the name that the
 	// deletion happened, and it stops looking for the real one.
-	if _, err := memory.Read(tool.paths, scope, name); errors.Is(err, memory.ErrNotFound) {
-		return okResult(fmt.Sprintf("No note named %q in %s, so there was nothing to forget.", name, scope))
+	if _, readErr := memory.Read(tool.paths, scope, name); readErr != nil {
+		if errors.Is(readErr, memory.ErrNotFound) {
+			return okResult(fmt.Sprintf("No note named %q in %s, so there was nothing to forget.", name, scope))
+		}
+		// Read-before-delete is a safety gate, not only an existence probe. If the
+		// note cannot be read, deleting it would destroy the only copy without the
+		// caller ever being able to inspect what the destructive tool removed.
+		return errorResult(fmt.Sprintf("Error: cannot read memory %q in %s before deleting it: %v", name, scope, readErr))
 	}
 	if err := memory.Forget(tool.paths, scope, name); err != nil {
 		return errorResult("Error: " + err.Error())

internal/tools/memory_test.go

@@ -156,6 +156,33 @@ func TestMemoryForgetReportsAbsenceRatherThanClaimingSuccess(t *testing.T) {
 	}
 }
 
+func TestMemoryForgetPreservesANoteThatCannotBeRead(t *testing.T) {
+	paths := memoryTestPaths(t)
+	if err := os.MkdirAll(paths.LocalDir, 0o700); err != nil {
+		t.Fatal(err)
+	}
+	notePath := filepath.Join(paths.LocalDir, "unreadable.md")
+	// Oversize input is a deterministic, cross-platform read failure; unlike
+	// permission bits, it behaves the same when tests run with elevated access.
+	if err := os.WriteFile(notePath, []byte(strings.Repeat("x", 70<<10)), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := memory.Read(paths, memory.ScopeLocal, "unreadable"); err == nil {
+		t.Fatal("oversized fixture unexpectedly remained readable")
+	}
+
+	result := NewMemoryForgetTool(paths).Run(context.Background(), map[string]any{"name": "unreadable"})
+	if result.Status != StatusError {
+		t.Fatalf("memory_forget deleted an unreadable note: %q", result.Output)
+	}
+	if !strings.Contains(strings.ToLower(result.Output), "cannot read") {
+		t.Fatalf("memory_forget hid the read failure: %q", result.Output)
+	}
+	if _, err := os.Stat(notePath); err != nil {
+		t.Fatalf("memory_forget removed the unreadable note: %v", err)
+	}
+}
+
 // One unreadable note must not empty the listing. memory.List deliberately
 // returns what it could read alongside the failures; the tool returning an error
 // instead threw that away, turning partial success back into total failure one