An unread memory file keeps its unflushed flag, is never overwritten, and is named when the index refuses

cmd/logos/index.go

@@ -193,6 +193,10 @@ func indexPass(ix *index.Index, w io.Writer, p *provider.Provider, embed string)
 	// nothing" cannot depend on a model being reachable.
 	mems, rescuedMems, edits, err := ix.SyncMemoryEdits(p, embed)
 	if err != nil {
+		// What the scan found before it refused is still true, and an
+		// unreadable file left out of this run would otherwise go unnamed
+		// behind the refusal.
+		printUnreadMemoryFiles(w, "", edits)
 		return err
 	}
 	printMemoryEdits(w, edits)

cmd/logos/index_refuses_unread_test.go

@@ -0,0 +1,46 @@
+package main
+
+import (
+	"os"
+	"path/filepath"
+	"runtime"
+	"strings"
+	"testing"
+)
+
+// A damaged digest elsewhere makes the index refuse, and the refusal returned
+// before the notice: the unreadable file the same scan found went unnamed, so
+// the one run that had found it said nothing about it.
+func TestAnUnreadableMemoryFileIsNamedWhenTheIndexRefuses(t *testing.T) {
+	if runtime.GOOS == "windows" || os.Geteuid() == 0 {
+		t.Skip("needs a file permission that stops its owner reading it")
+	}
+	vaultDir := t.TempDir()
+	t.Setenv("LOGOS_VAULT", vaultDir)
+	t.Setenv("LOGOS_EMBED", "off")
+	if err := memoryCmd([]string{"add", "--project", "shop", "Deploys go out on Tuesdays"}); err != nil {
+		t.Fatalf("memory add: %v", err)
+	}
+	matches, err := filepath.Glob(filepath.Join(vaultDir, "memories", "fact", "*.md"))
+	if err != nil || len(matches) != 1 {
+		t.Fatalf("got %v per-memory fact files (%v), want 1", matches, err)
+	}
+	locked := matches[0]
+	if err := os.Chmod(locked, 0); err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { os.Chmod(locked, 0o644) })
+	torn := filepath.Join(vaultDir, "memories", "context.md")
+	if err := os.WriteFile(torn, []byte("- Shipping in November <!-- uid=0123"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+
+	var ierr error
+	out := captureStdout(t, func() { ierr = runIndex(false) })
+	if ierr == nil {
+		t.Fatal("index accepted a torn context.md")
+	}
+	if want := "memories/fact/" + filepath.Base(locked); !strings.Contains(out, want) {
+		t.Errorf("the refusing index did not name the file it could not read; want %q in:\n%s", want, out)
+	}
+}

internal/memory/unreadable_file_test.go

@@ -0,0 +1,79 @@
+package memory
+
+import (
+	"bytes"
+	"database/sql"
+	"os"
+	"runtime"
+	"testing"
+)
+
+// storeUnreadable stores one fact and makes its own file unreadable, the
+// state a chmod, a sync client mid-download or a disk error leaves behind.
+func storeUnreadable(t *testing.T) (db *sql.DB, dir, path string, id int64) {
+	t.Helper()
+	if runtime.GOOS == "windows" || os.Getuid() == 0 {
+		t.Skip("needs a file its owner cannot read")
+	}
+	db, dir = vaultDB(t)
+	r, err := Store(db, nil, "", &Memory{Text: "Deploys go out on Tuesdays", Kind: Fact, Source: "manual"})
+	if err != nil {
+		t.Fatal(err)
+	}
+	var uid string
+	if err := db.QueryRow(`SELECT uid FROM memories WHERE id = ?`, r.ID).Scan(&uid); err != nil {
+		t.Fatal(err)
+	}
+	path = memoryFilePath(dir, Fact, uid)
+	if err := os.Chmod(path, 0); err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { os.Chmod(path, 0o600) })
+	return db, dir, path, r.ID
+}
+
+// An unreadable file confirms nothing about its memory. Counted as found, it
+// cleared the unflushed flag of a memory no read had seen in the vault, and
+// doctor stopped reporting a row that one more wipe would lose.
+func TestAnUnreadableMemoryFileKeepsItsUnflushedFlag(t *testing.T) {
+	db, dir, _, id := storeUnreadable(t)
+	if _, err := db.Exec(`UPDATE memories SET unflushed = 1 WHERE id = ?`, id); err != nil {
+		t.Fatal(err)
+	}
+	if _, _, _, err := ImportEdits(db, nil, "", dir); err != nil {
+		t.Fatal(err)
+	}
+	var unflushed int
+	if err := db.QueryRow(`SELECT unflushed FROM memories WHERE id = ?`, id).Scan(&unflushed); err != nil {
+		t.Fatalf("the memory behind an unreadable file is gone: %v", err)
+	}
+	if unflushed != 1 {
+		t.Error("an index cleared the unflushed flag of a memory whose file it could not read")
+	}
+}
+
+// The hand-edit guard compares the file with its stamp. A file it could not
+// read was taken as nothing to compare, and the rewrite replaced whatever a
+// person had put there.
+func TestAMemoryFileThatCannotBeReadIsNotOverwritten(t *testing.T) {
+	db, _, path, _ := storeUnreadable(t)
+	os.Chmod(path, 0o600)
+	before, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	os.Chmod(path, 0)
+
+	// The same text again reinforces the memory, which rewrites its file.
+	if _, err := Store(db, nil, "", &Memory{Text: "Deploys go out on Tuesdays", Kind: Fact, Source: "manual"}); err == nil {
+		t.Error("a store rewrote a memory file it could not read, with no error")
+	}
+	os.Chmod(path, 0o600)
+	after, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !bytes.Equal(before, after) {
+		t.Error("the unreadable memory file was replaced")
+	}
+}

internal/memory/vaultstore.go

@@ -667,7 +667,14 @@ func writeMemoryFile(db *sql.DB, dir string, kind Kind, m Memory) error {
 		return fmt.Errorf("memory #%d has no uid to file it under", m.ID)
 	}
 	path := memoryFilePath(dir, kind, m.Uid)
-	if onDisk, err := os.ReadFile(path); err == nil {
+	onDisk, err := os.ReadFile(path)
+	// A file there that cannot be read may hold a hand edit nobody can check,
+	// and the index keeps its row now; replacing it would lose that edit with
+	// no refusal.
+	if err != nil && !os.IsNotExist(err) {
+		return fmt.Errorf("memories/%s/%s.md could not be read to check it for hand edits, so it was not rewritten: %w", kind, m.Uid, err)
+	}
+	if err == nil {
 		last, stamped := fileStampSeen(db, m.Uid)
 		pending, pendingStamped := fileStampSeen(db, pendingUID(m.Uid))
 		if mine, claimed := ours(sha256.Sum256(onDisk), last, stamped, pending, pendingStamped); claimed && !mine {
@@ -852,6 +859,11 @@ func importInto(db *sql.DB, p *provider.Provider, embedModel, dir string, edits
 	}
 
 	keep := map[int64]bool{}
+	// unread rows are named by a file nobody could read, or one holding no
+	// memory line: kept from the reap like keep's, but not counted as found
+	// in the vault, since nothing confirmed them there and an unflushed flag
+	// has to stay set for doctor and the next index.
+	unread := map[int64]bool{}
 
 	// Only kinds whose file was read and found intact are authoritative. A kind
 	// whose file is missing or truncated says nothing about what the user wants
@@ -895,7 +907,7 @@ func importInto(db *sql.DB, p *provider.Provider, embedModel, dir string, edits
 			var id int64
 			uid := strings.TrimSuffix(filepath.Base(rel), ".md")
 			if db.QueryRow("SELECT id FROM memories WHERE uid = ? AND kind = ?", uid, string(kind)).Scan(&id) == nil {
-				keep[id] = true
+				unread[id] = true
 			}
 		}
 
@@ -986,6 +998,9 @@ func importInto(db *sql.DB, p *provider.Provider, embedModel, dir string, edits
 			rows.Close()
 			return imported, rescued, err
 		}
+		if unread[id] {
+			continue
+		}
 		if keep[id] {
 			// Already recovered — not by the rescue below, but by its own
 			// per-memory file, found and read during the directory scan