From debbead6e21c5ff3cb003794db10829d8fee4360 Mon Sep 17 00:00:00 2001 From: Coder8124 Date: Sun, 1 Nov 2026 11:49:09 -0800 Subject: [PATCH 1/6] BUG: An unreadable memory file is named by the index that left it out --- cmd/logos/index.go | 7 ++++ cmd/logos/index_unreadable_memory_test.go | 48 +++++++++++++++++++++++ internal/memory/vaultstore.go | 17 ++++++++ 3 files changed, 72 insertions(+) create mode 100644 cmd/logos/index_unreadable_memory_test.go diff --git a/cmd/logos/index.go b/cmd/logos/index.go index f33143b..d103a57 100644 --- a/cmd/logos/index.go +++ b/cmd/logos/index.go @@ -395,6 +395,13 @@ func printConflictCopies(w io.Writer, prefix string, walked []string, e memory.E fmt.Fprintf(w, "%sset aside %d sync conflict %s of a memory that is no longer current — delete %s, or rename one to its original name to bring the memory back: %s\n", prefix, n, plural2(n, "copy", "copies"), plural2(n, "it", "them"), terminalSafe(strings.Join(e.Orphaned, ", "), 2000)) } + // Said here, with the other files an index left out, because it is the + // same promise: a file on disk that is not in memory is named, or the + // counts read as a vault that simply has fewer memories. + if n := len(e.Unreadable); n > 0 { + fmt.Fprintf(w, "%scould not read %d memory %s, left out of memory until %s readable: %s\n", + prefix, n, plural2(n, "file", "files"), plural2(n, "it is", "they are"), terminalSafe(strings.Join(e.Unreadable, ", "), 2000)) + } } func plural2(n int, one, many string) string { diff --git a/cmd/logos/index_unreadable_memory_test.go b/cmd/logos/index_unreadable_memory_test.go new file mode 100644 index 0000000..8929c44 --- /dev/null +++ b/cmd/logos/index_unreadable_memory_test.go @@ -0,0 +1,48 @@ +package main + +import ( + "os" + "path/filepath" + "runtime" + "strings" + "testing" +) + +// A memory file the index cannot read was skipped with a bare continue: after +// a rebuild `logos index` counted one memory fewer, the digest was rewritten +// without it, and nothing named the file still sitting on disk. +func TestAnUnreadableMemoryFileIsNamedByTheIndexThatLeftItOut(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") + for _, text := range []string{"Decided to use Stripe for payments", "Deploys go out on Tuesdays"} { + if err := memoryCmd([]string{"add", "--project", "shop", text}); err != nil { + t.Fatalf("memory add: %v", err) + } + } + matches, err := filepath.Glob(filepath.Join(vaultDir, "memories", "fact", "*.md")) + if err != nil || len(matches) != 2 { + t.Fatalf("got %v per-memory fact files (%v), want 2", matches, err) + } + locked := matches[0] + if err := os.Chmod(locked, 0); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Chmod(locked, 0o644) }) + if err := os.RemoveAll(filepath.Join(vaultDir, ".logos")); err != nil { + t.Fatal(err) + } + + out := captureStdout(t, func() { + if err := runIndex(false); err != nil { + t.Fatal(err) + } + }) + want := "could not read 1 memory file, left out of memory until it is readable: memories/fact/" + filepath.Base(locked) + ": permission denied" + if !strings.Contains(out, want) { + t.Errorf("index did not name the file it could not read; want %q in:\n%s", want, out) + } +} diff --git a/internal/memory/vaultstore.go b/internal/memory/vaultstore.go index b6220aa..95bf519 100644 --- a/internal/memory/vaultstore.go +++ b/internal/memory/vaultstore.go @@ -6,6 +6,7 @@ import ( "encoding/hex" "errors" "fmt" + "io/fs" "os" "path/filepath" "sort" @@ -747,6 +748,11 @@ type Edits struct { // client made a second one, and reading that copy as live brought the // forgotten memory back on the next import without a word. SetAside, Orphaned []string + // Unreadable are memory files under memories// that could not be + // read, each "path: why". Such a file drops its memory from the index and + // the digest for as long as it stays unreadable, and nothing else says so: + // the counts read as a vault with one memory fewer, not one memory hidden. + Unreadable []string } // ImportEdits is Import, also saying which memories a hand edit made or @@ -1254,6 +1260,17 @@ func readKindFiles(db *sql.DB, dir string, kind Kind, e *Edits) (raw string, exi // Unreadable, not missing — treated the same as linesElsewhere // treats one: costs this memory its place in the digest, never // a reason to call the whole kind damaged over one bad file. + // Recorded, since a memory that vanishes with no word is the + // silent loss invariant 4 rules out. + if e != nil { + rel := filepath.ToSlash(filepath.Join(Dir, string(kind), en.Name())) + why := rerr + var pe *fs.PathError + if errors.As(rerr, &pe) { + why = pe.Err // the path is said once, vault-relative + } + e.Unreadable = append(e.Unreadable, rel+": "+why.Error()) + } continue } have[strings.TrimSuffix(en.Name(), ".md")] = true From f8a1570debafccec0081ce31364db3cdc5daa5de Mon Sep 17 00:00:00 2001 From: Coder8124 Date: Sun, 1 Nov 2026 11:49:09 -0800 Subject: [PATCH 2/6] BUG: A memory write killed before its hash is recorded is not refused as a hand edit next time --- internal/memory/killed_write_test.go | 44 +++++++++ internal/memory/memory.go | 7 +- internal/memory/vaultstore.go | 143 ++++++++++++++++++++------- 3 files changed, 156 insertions(+), 38 deletions(-) create mode 100644 internal/memory/killed_write_test.go diff --git a/internal/memory/killed_write_test.go b/internal/memory/killed_write_test.go new file mode 100644 index 0000000..6f50c72 --- /dev/null +++ b/internal/memory/killed_write_test.go @@ -0,0 +1,44 @@ +package memory + +import ( + "strings" + "testing" +) + +// A process killed after a memory file or digest reached the disk, and before +// its hash was recorded, left a file the stamp did not claim. The next store +// took logos's own write for a hand edit and refused, and so did every one +// after it. Seen once in the chaos tier's killed memory add (#371). +func TestAWriteKilledBeforeItsStampIsNotTakenForAHandEdit(t *testing.T) { + for name, file := range map[string]string{"digest": "/fact.md", "own file": ".md"} { + t.Run(name, func(t *testing.T) { + db, _ := vaultDB(t) + // The same text each time: after the first, each store is a + // reinforcement that rewrites the memory's own file and the + // digest, both already stamped, each through its hand-edit guard. + store := func() error { + _, err := Store(db, nil, "", &Memory{Text: "Deploys go out on Tuesdays", Kind: Fact, Source: "manual"}) + return err + } + if err := store(); err != nil { + t.Fatal(err) + } + // The own file is written first and ends in ".md"; "/fact.md" + // matches only the digest. + vaultWritten = func(path string) { + if strings.HasSuffix(path, file) { + panic("killed") + } + } + func() { + defer func() { recover() }() + store() + }() + vaultWritten = func(string) {} + + if err := store(); err != nil { + t.Fatalf("the store after a killed write was refused: %v", err) + } + }) + } +} diff --git a/internal/memory/memory.go b/internal/memory/memory.go index 89a8cdc..79dba2c 100644 --- a/internal/memory/memory.go +++ b/internal/memory/memory.go @@ -997,8 +997,11 @@ func carryRecallTimes(db *sql.DB, kind Kind, ms []Memory, now int64) error { errs = append(errs, fmt.Errorf("memories/%s/%s.md has no uses= to update", kind, m.Uid)) continue } - last, stamped := fileStampSeen(db, m.Uid) - ours := stamped && sha256.Sum256(raw) == last + last, seen := fileStampSeen(db, m.Uid) + ours := seen && stamped(last, sha256.Sum256(raw)) + if ours { + fileStampPending(db, m.Uid, sha256.Sum256([]byte(patched))) + } if err := vault.WriteAtomic(path, []byte(patched)); err != nil { errs = append(errs, fmt.Errorf("memory #%d: %w", m.ID, err)) continue diff --git a/internal/memory/vaultstore.go b/internal/memory/vaultstore.go index 95bf519..8cf4863 100644 --- a/internal/memory/vaultstore.go +++ b/internal/memory/vaultstore.go @@ -186,26 +186,62 @@ func stamp(db *sql.DB, kind Kind, sum [sha256.Size]byte) { ON CONFLICT(kind) DO UPDATE SET sha = excluded.sha`, string(kind), hex.EncodeToString(sum[:])) } -// stampSeen is the hash of the digest this process (or an earlier one, since -// the table outlives any one process) last wrote for this kind, and whether -// one was ever recorded — flushLocked's hand-edit guard needs that second -// value, since a plain bool can't tell "recorded one, but it doesn't match" -// apart from "no stamp yet", and a kind's first-ever write must never refuse. -func stampSeen(db *sql.DB, kind Kind) ([sha256.Size]byte, bool) { +// stampSeen is every hash of this kind's digest that counts as logos's own +// write, and whether one was ever recorded — flushLocked's hand-edit guard +// needs that second value, since an empty answer can't tell "recorded one, but +// it doesn't match" apart from "no stamp yet", and a kind's first-ever write +// must never refuse. Usually one hash; more while a write is in flight (see +// stampPending). +func stampSeen(db *sql.DB, kind Kind) ([][sha256.Size]byte, bool) { stampMu.Lock() defer stampMu.Unlock() - var hexSum string - err := db.QueryRow(`SELECT sha FROM digest_stamps WHERE kind = ?`, string(kind)).Scan(&hexSum) - if err != nil { - return [sha256.Size]byte{}, false + var hexSums string + if err := db.QueryRow(`SELECT sha FROM digest_stamps WHERE kind = ?`, string(kind)).Scan(&hexSums); err != nil { + return nil, false } - raw, err := hex.DecodeString(hexSum) - if err != nil || len(raw) != sha256.Size { - return [sha256.Size]byte{}, false + return parseStamps(hexSums) +} + +// stampPending adds the hash of the digest about to be written to the kind's +// stamp, beside the one already there, before the write. A process killed +// between the write and stamp's record of it used to leave the new file on +// disk and only the old hash in the table, and the next write refused logos's +// own file as a hand edit — every later memory add of that kind failed until +// someone ran `logos index`. With both hashes held across the write, a kill on +// either side of the rename leaves a file the stamp still claims. stamp +// replaces the pair once the write has landed. No stamp yet means no guard to +// satisfy, so nothing is added. +func stampPending(db *sql.DB, kind Kind, next [sha256.Size]byte) { + stampMu.Lock() + defer stampMu.Unlock() + _, _ = db.Exec(`UPDATE digest_stamps SET sha = sha || ',' || ? WHERE kind = ?`, hex.EncodeToString(next[:]), string(kind)) +} + +// parseStamps reads a stamp column: one hex hash, or several joined by commas +// while a write is in flight. A value that does not parse is treated as no +// stamp, the same as a missing row — a missed detection, never a refusal. +func parseStamps(hexSums string) ([][sha256.Size]byte, bool) { + var sums [][sha256.Size]byte + for _, h := range strings.Split(hexSums, ",") { + raw, err := hex.DecodeString(h) + if err != nil || len(raw) != sha256.Size { + return nil, false + } + var sum [sha256.Size]byte + copy(sum[:], raw) + sums = append(sums, sum) } - var sum [sha256.Size]byte - copy(sum[:], raw) - return sum, true + return sums, true +} + +// stamped reports whether sum is one of the hashes a stamp claims. +func stamped(sums [][sha256.Size]byte, sum [sha256.Size]byte) bool { + for _, s := range sums { + if s == sum { + return true + } + } + return false } // dropStamps forgets every kind's claim for this vault, so a handle rebound @@ -238,23 +274,29 @@ func fileStamp(db *sql.DB, uid string, sum [sha256.Size]byte) { ON CONFLICT(uid) DO UPDATE SET sha = excluded.sha`, uid, hex.EncodeToString(sum[:])) } -func fileStampSeen(db *sql.DB, uid string) ([sha256.Size]byte, bool) { +func fileStampSeen(db *sql.DB, uid string) ([][sha256.Size]byte, bool) { stampMu.Lock() defer stampMu.Unlock() - var hexSum string - err := db.QueryRow(`SELECT sha FROM memory_file_stamps WHERE uid = ?`, uid).Scan(&hexSum) - if err != nil { - return [sha256.Size]byte{}, false + var hexSums string + if err := db.QueryRow(`SELECT sha FROM memory_file_stamps WHERE uid = ?`, uid).Scan(&hexSums); err != nil { + return nil, false } - raw, err := hex.DecodeString(hexSum) - if err != nil || len(raw) != sha256.Size { - return [sha256.Size]byte{}, false - } - var sum [sha256.Size]byte - copy(sum[:], raw) - return sum, true + return parseStamps(hexSums) +} + +// fileStampPending is stampPending for one memory's own file, for the same +// kill between the write and its record. +func fileStampPending(db *sql.DB, uid string, next [sha256.Size]byte) { + stampMu.Lock() + defer stampMu.Unlock() + _, _ = db.Exec(`UPDATE memory_file_stamps SET sha = sha || ',' || ? WHERE uid = ?`, hex.EncodeToString(next[:]), uid) } +// vaultWritten runs after a memory file or digest is written and before its +// stamp is recorded. A test panics there to stand for a process killed in +// that gap. +var vaultWritten = func(path string) {} + // dropFileStamp forgets one uid's claim, used when its file is removed so a // later uid collision (same file path reused) never trusts a stale hash. func dropFileStamp(db *sql.DB, uid string) { @@ -304,12 +346,19 @@ func flushLocked(db *sql.DB, kind Kind) error { // directly, never through this guard. if last, ok := stampSeen(db, kind); ok { if onDisk, err := os.ReadFile(filepath.Join(dir, Dir, string(kind)+".md")); err == nil { - if sha256.Sum256(onDisk) != last { + if !stamped(last, sha256.Sum256(onDisk)) { return fmt.Errorf("memories/%s.md was hand-edited since logos last wrote it; it is a generated digest now — edit the memory's own file under memories/%s/ instead, then run `logos index` to refresh this one", kind, kind) } } } - if err := ExportKind(db, dir, kind); err != nil { + content, err := renderKindDigest(db, kind) + if err == nil { + if content != "" { + stampPending(db, kind, sha256.Sum256([]byte(content))) + } + err = writeKindDigest(dir, kind, content) + } + if err != nil { // Record which rows the failed export left behind, so the split state // outlives the one call that was told about it. The caller here gets an // honest error and then exits; everyone after it — recall, doctor, the @@ -317,6 +366,7 @@ func flushLocked(db *sql.DB, kind Kind) error { markUnflushed(db, dir, kind) return fmt.Errorf("memory saved to the cache but not to the vault: %w", err) } + vaultWritten(filepath.Join(dir, Dir, string(kind)+".md")) // The file on disk is now this kind, whole. Whatever an earlier failure // stranded went out with it, because the export rewrites the entire file // rather than appending — which is also why a single later write of any @@ -467,11 +517,22 @@ func Export(db *sql.DB, dir string) error { // make quarantine theatre — reviewable in principle, already believed in // practice. They start appearing here the moment Accept clears the flag. func ExportKind(db *sql.DB, dir string, kind Kind) error { + content, err := renderKindDigest(db, kind) + if err != nil { + return err + } + return writeKindDigest(dir, kind, content) +} + +// renderKindDigest is the digest ExportKind would write for one kind, or "" +// for a kind with no current memories. Split from the write so flushLocked can +// stamp the bytes before they reach the disk. +func renderKindDigest(db *sql.DB, kind Kind) (string, error) { rows, err := db.Query( `SELECT id, text, salience, confidence, project, source, agent, created, last_used, uses, pin, auto_accepted, commit_sha, uid, supersedes FROM memories WHERE kind = ? AND superseded = 0 AND quarantined = 0 ORDER BY created, id`, string(kind)) if err != nil { - return err + return "", err } defer rows.Close() @@ -481,17 +542,25 @@ func ExportKind(db *sql.DB, dir string, kind Kind) error { var project, source, agent sql.NullString if err := rows.Scan(&m.ID, &m.Text, &m.Salience, &m.Confidence, &project, &source, &agent, &m.Created, &m.LastUsed, &m.Uses, &m.Pin, &m.AutoAccepted, &m.Commit, &m.Uid, &m.Supersedes); err != nil { - return err + return "", err } m.Kind, m.Project, m.Source, m.Agent = kind, project.String, source.String, agent.String mems = append(mems, m) } if err := rows.Err(); err != nil { - return err + return "", err } + if len(mems) == 0 { + return "", nil + } + return renderKind(kind, mems), nil +} +// writeKindDigest writes a digest renderKindDigest produced, or removes the +// file for a kind with none. +func writeKindDigest(dir string, kind Kind, content string) error { path := filepath.Join(dir, Dir, string(kind)+".md") - if len(mems) == 0 { + if content == "" { // Remove rather than leave an empty file: a kind nobody uses should not // clutter the vault, and its absence is not ambiguous. if err := os.Remove(path); err != nil && !os.IsNotExist(err) { @@ -499,7 +568,7 @@ func ExportKind(db *sql.DB, dir string, kind Kind) error { } return nil } - return vault.WriteAtomic(path, []byte(renderKind(kind, mems))) + return vault.WriteAtomic(path, []byte(content)) } // generatedDigestBanner marks a kind's flat file as logos's own rendered @@ -607,15 +676,17 @@ func writeMemoryFile(db *sql.DB, dir string, kind Kind, m Memory) error { path := memoryFilePath(dir, kind, m.Uid) if last, ok := fileStampSeen(db, m.Uid); ok { if onDisk, err := os.ReadFile(path); err == nil { - if sha256.Sum256(onDisk) != last { + if !stamped(last, sha256.Sum256(onDisk)) { return fmt.Errorf("memories/%s/%s.md was hand-edited since logos last wrote it — correct the memory with `logos memory correct`, or edit the file and run `logos index`", kind, m.Uid) } } } content := []byte(renderMemoryFile(kind, m)) + fileStampPending(db, m.Uid, sha256.Sum256(content)) if err := vault.WriteAtomic(path, content); err != nil { return err } + vaultWritten(path) fileStamp(db, m.Uid, sha256.Sum256(content)) return nil } From 3da1ba263be5a2040e239c7ef3a9536666252eb7 Mon Sep 17 00:00:00 2001 From: Coder8124 Date: Sun, 1 Nov 2026 11:53:54 -0800 Subject: [PATCH 3/6] BUG: An index does not forget a memory, or delete its file, because the file could not be read --- cmd/logos/index.go | 4 +- cmd/logos/index_unreadable_memory_test.go | 53 ++++++++++++++++++++++- internal/memory/vaultstore.go | 21 +++++++-- 3 files changed, 72 insertions(+), 6 deletions(-) diff --git a/cmd/logos/index.go b/cmd/logos/index.go index d103a57..b5c31af 100644 --- a/cmd/logos/index.go +++ b/cmd/logos/index.go @@ -399,8 +399,8 @@ func printConflictCopies(w io.Writer, prefix string, walked []string, e memory.E // same promise: a file on disk that is not in memory is named, or the // counts read as a vault that simply has fewer memories. if n := len(e.Unreadable); n > 0 { - fmt.Fprintf(w, "%scould not read %d memory %s, left out of memory until %s readable: %s\n", - prefix, n, plural2(n, "file", "files"), plural2(n, "it is", "they are"), terminalSafe(strings.Join(e.Unreadable, ", "), 2000)) + fmt.Fprintf(w, "%scould not read %d memory %s; a memory the index already has is kept, but none is read from %s until %s readable: %s\n", + prefix, n, plural2(n, "file", "files"), plural2(n, "it", "them"), plural2(n, "it is", "they are"), terminalSafe(strings.Join(e.Unreadable, ", "), 2000)) } } diff --git a/cmd/logos/index_unreadable_memory_test.go b/cmd/logos/index_unreadable_memory_test.go index 8929c44..12672ed 100644 --- a/cmd/logos/index_unreadable_memory_test.go +++ b/cmd/logos/index_unreadable_memory_test.go @@ -41,8 +41,59 @@ func TestAnUnreadableMemoryFileIsNamedByTheIndexThatLeftItOut(t *testing.T) { t.Fatal(err) } }) - want := "could not read 1 memory file, left out of memory until it is readable: memories/fact/" + filepath.Base(locked) + ": permission denied" + want := "could not read 1 memory file; a memory the index already has is kept, but none is read from it until it is readable: memories/fact/" + filepath.Base(locked) + ": permission denied" if !strings.Contains(out, want) { t.Errorf("index did not name the file it could not read; want %q in:\n%s", want, out) } } + +// Over an index that already had the memory, an unreadable file read as a line +// the user deleted: the index forgot the memory and removed the file it could +// not read, so fixing the permission afterwards brought nothing back. +func TestAnUnreadableMemoryFileIsNotForgottenByTheNextIndex(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") + for _, text := range []string{"Decided to use Stripe for payments", "Deploys go out on Tuesdays"} { + if err := memoryCmd([]string{"add", "--project", "shop", text}); err != nil { + t.Fatalf("memory add: %v", err) + } + } + matches, err := filepath.Glob(filepath.Join(vaultDir, "memories", "fact", "*.md")) + if err != nil || len(matches) != 2 { + t.Fatalf("got %v per-memory fact files (%v), want 2", matches, err) + } + locked := matches[0] + if err := os.Chmod(locked, 0); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Chmod(locked, 0o644) }) + + out := captureStdout(t, func() { + if err := runIndex(false); err != nil { + t.Fatal(err) + } + }) + if strings.Contains(out, "forgot") { + t.Errorf("index forgot the memory whose file it could not read:\n%s", out) + } + if _, err := os.Lstat(locked); err != nil { + t.Fatalf("index removed the memory file it could not read: %v", err) + } + if err := os.Chmod(locked, 0o644); err != nil { + t.Fatal(err) + } + listed := captureStdout(t, func() { + if err := memoryCmd(nil); err != nil { + t.Fatal(err) + } + }) + for _, text := range []string{"Stripe", "Tuesdays"} { + if !strings.Contains(listed, text) { + t.Errorf("%q is not listed after the file was readable again:\n%s", text, listed) + } + } +} diff --git a/internal/memory/vaultstore.go b/internal/memory/vaultstore.go index 8cf4863..eda8251 100644 --- a/internal/memory/vaultstore.go +++ b/internal/memory/vaultstore.go @@ -820,10 +820,14 @@ type Edits struct { // forgotten memory back on the next import without a word. SetAside, Orphaned []string // Unreadable are memory files under memories// that could not be - // read, each "path: why". Such a file drops its memory from the index and - // the digest for as long as it stays unreadable, and nothing else says so: - // the counts read as a vault with one memory fewer, not one memory hidden. + // read, each "path: why". A rebuilt index has no row for such a file's + // memory until it is readable again, and nothing else says so: the counts + // read as a vault with one memory fewer, not one memory hidden. Unreadable []string + // unreadableUIDs are the same files' uids, so the reaping in ImportEdits + // keeps the rows they name: the file is there, only unreadable, and + // forgetting its memory deleted the file with it. + unreadableUIDs []string } // ImportEdits is Import, also saying which memories a hand edit made or @@ -929,6 +933,13 @@ func importInto(db *sql.DB, p *provider.Provider, embedModel, dir string, edits // A damaged file is reported and never acted on. Importing what survived // would be the same silent deletion this guard exists to prevent, and the // user needs to know before they overwrite the file by writing a new memory. + for _, uid := range edits.unreadableUIDs { + var id int64 + if db.QueryRow("SELECT id FROM memories WHERE uid = ? AND superseded = 0", uid).Scan(&id) == nil { + keep[id] = true + } + } + if len(damaged) > 0 { return imported, rescued, fmt.Errorf( "refusing to import an incomplete memory store (%s); "+ @@ -1341,7 +1352,11 @@ func readKindFiles(db *sql.DB, dir string, kind Kind, e *Edits) (raw string, exi why = pe.Err // the path is said once, vault-relative } e.Unreadable = append(e.Unreadable, rel+": "+why.Error()) + e.unreadableUIDs = append(e.unreadableUIDs, strings.TrimSuffix(en.Name(), ".md")) } + // Not migrated over either: a legacy line with this uid would + // be written onto the file nobody could read. + have[strings.TrimSuffix(en.Name(), ".md")] = true continue } have[strings.TrimSuffix(en.Name(), ".md")] = true From 15b9a44acd24035887aea94fd571db5f4a147c37 Mon Sep 17 00:00:00 2001 From: Coder8124 Date: Mon, 2 Nov 2026 15:29:06 -0800 Subject: [PATCH 4/6] BUG: A memory write that keeps failing holds one pending hash, not one per attempt --- internal/memory/killed_write_test.go | 31 ++++++++++++++++++++++++++++ internal/memory/memory.go | 8 ++++--- internal/memory/vaultstore.go | 8 +++++-- 3 files changed, 42 insertions(+), 5 deletions(-) diff --git a/internal/memory/killed_write_test.go b/internal/memory/killed_write_test.go index 6f50c72..365fd27 100644 --- a/internal/memory/killed_write_test.go +++ b/internal/memory/killed_write_test.go @@ -1,6 +1,7 @@ package memory import ( + "crypto/sha256" "strings" "testing" ) @@ -25,6 +26,7 @@ func TestAWriteKilledBeforeItsStampIsNotTakenForAHandEdit(t *testing.T) { } // The own file is written first and ends in ".md"; "/fact.md" // matches only the digest. + t.Cleanup(func() { vaultWritten = func(string) {} }) vaultWritten = func(path string) { if strings.HasSuffix(path, file) { panic("killed") @@ -42,3 +44,32 @@ func TestAWriteKilledBeforeItsStampIsNotTakenForAHandEdit(t *testing.T) { }) } } + +// A write that keeps failing — a read-only or full vault — calls stampPending +// once per attempt. Appended beside whatever was there, the column grew a hash +// per attempt, and every one of them stayed a claim the file was logos's own. +func TestARepeatedlyFailingWriteHoldsOnlyOnePendingHash(t *testing.T) { + db, _ := 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) + } + for i := 0; i < 3; i++ { + next := sha256.Sum256([]byte{byte(i)}) + stampPending(db, Fact, next) + fileStampPending(db, uid, next) + } + for table, where := range map[string]string{"digest_stamps": "kind = 'fact'", "memory_file_stamps": "uid = '" + uid + "'"} { + var sha string + if err := db.QueryRow(`SELECT sha FROM ` + table + ` WHERE ` + where).Scan(&sha); err != nil { + t.Fatal(err) + } + if n := strings.Count(sha, ",") + 1; n != 2 { + t.Errorf("%s holds %d hashes after three failed writes, want the confirmed one and the last pending one", table, n) + } + } +} diff --git a/internal/memory/memory.go b/internal/memory/memory.go index 79dba2c..5fc0e7a 100644 --- a/internal/memory/memory.go +++ b/internal/memory/memory.go @@ -999,15 +999,17 @@ func carryRecallTimes(db *sql.DB, kind Kind, ms []Memory, now int64) error { } last, seen := fileStampSeen(db, m.Uid) ours := seen && stamped(last, sha256.Sum256(raw)) + out := []byte(patched) + sum := sha256.Sum256(out) if ours { - fileStampPending(db, m.Uid, sha256.Sum256([]byte(patched))) + fileStampPending(db, m.Uid, sum) } - if err := vault.WriteAtomic(path, []byte(patched)); err != nil { + if err := vault.WriteAtomic(path, out); err != nil { errs = append(errs, fmt.Errorf("memory #%d: %w", m.ID, err)) continue } if ours { - fileStamp(db, m.Uid, sha256.Sum256([]byte(patched))) + fileStamp(db, m.Uid, sum) } } return errors.Join(errs...) diff --git a/internal/memory/vaultstore.go b/internal/memory/vaultstore.go index eda8251..ead15e3 100644 --- a/internal/memory/vaultstore.go +++ b/internal/memory/vaultstore.go @@ -211,10 +211,14 @@ func stampSeen(db *sql.DB, kind Kind) ([][sha256.Size]byte, bool) { // either side of the rename leaves a file the stamp still claims. stamp // replaces the pair once the write has landed. No stamp yet means no guard to // satisfy, so nothing is added. +// +// Beside the last confirmed hash, never beside an earlier pending one: a write +// that keeps failing (a read-only or full vault) would otherwise grow the +// column by a hash per attempt, every one of them trusted. func stampPending(db *sql.DB, kind Kind, next [sha256.Size]byte) { stampMu.Lock() defer stampMu.Unlock() - _, _ = db.Exec(`UPDATE digest_stamps SET sha = sha || ',' || ? WHERE kind = ?`, hex.EncodeToString(next[:]), string(kind)) + _, _ = db.Exec(`UPDATE digest_stamps SET sha = substr(sha, 1, 64) || ',' || ? WHERE kind = ?`, hex.EncodeToString(next[:]), string(kind)) } // parseStamps reads a stamp column: one hex hash, or several joined by commas @@ -289,7 +293,7 @@ func fileStampSeen(db *sql.DB, uid string) ([][sha256.Size]byte, bool) { func fileStampPending(db *sql.DB, uid string, next [sha256.Size]byte) { stampMu.Lock() defer stampMu.Unlock() - _, _ = db.Exec(`UPDATE memory_file_stamps SET sha = sha || ',' || ? WHERE uid = ?`, hex.EncodeToString(next[:]), uid) + _, _ = db.Exec(`UPDATE memory_file_stamps SET sha = substr(sha, 1, 64) || ',' || ? WHERE uid = ?`, hex.EncodeToString(next[:]), uid) } // vaultWritten runs after a memory file or digest is written and before its From 57fc4c29c73d89acbbd9cf4a8ffa99fc035eb802 Mon Sep 17 00:00:00 2001 From: Coder8124 Date: Mon, 2 Nov 2026 15:29:06 -0800 Subject: [PATCH 5/6] BUG: A memory file that cannot be read keeps its unflushed flag and is not overwritten --- internal/memory/unreadable_file_test.go | 79 +++++++++++++++++++++++++ internal/memory/vaultstore.go | 41 +++++++++---- 2 files changed, 107 insertions(+), 13 deletions(-) create mode 100644 internal/memory/unreadable_file_test.go diff --git a/internal/memory/unreadable_file_test.go b/internal/memory/unreadable_file_test.go new file mode 100644 index 0000000..cf7374e --- /dev/null +++ b/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") + } +} diff --git a/internal/memory/vaultstore.go b/internal/memory/vaultstore.go index ead15e3..1db7280 100644 --- a/internal/memory/vaultstore.go +++ b/internal/memory/vaultstore.go @@ -679,19 +679,25 @@ func writeMemoryFile(db *sql.DB, dir string, kind Kind, m Memory) error { } path := memoryFilePath(dir, kind, m.Uid) if last, ok := fileStampSeen(db, m.Uid); ok { - if onDisk, err := os.ReadFile(path); err == nil { - if !stamped(last, sha256.Sum256(onDisk)) { - return fmt.Errorf("memories/%s/%s.md was hand-edited since logos last wrote it — correct the memory with `logos memory correct`, or edit the file and run `logos index`", kind, m.Uid) - } + 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 && !stamped(last, sha256.Sum256(onDisk)) { + return fmt.Errorf("memories/%s/%s.md was hand-edited since logos last wrote it — correct the memory with `logos memory correct`, or edit the file and run `logos index`", kind, m.Uid) } } content := []byte(renderMemoryFile(kind, m)) - fileStampPending(db, m.Uid, sha256.Sum256(content)) + sum := sha256.Sum256(content) + fileStampPending(db, m.Uid, sum) if err := vault.WriteAtomic(path, content); err != nil { return err } vaultWritten(path) - fileStamp(db, m.Uid, sha256.Sum256(content)) + fileStamp(db, m.Uid, sum) return nil } @@ -934,16 +940,21 @@ func importInto(db *sql.DB, p *provider.Provider, embedModel, dir string, edits authoritative[kind] = true } - // A damaged file is reported and never acted on. Importing what survived - // would be the same silent deletion this guard exists to prevent, and the - // user needs to know before they overwrite the file by writing a new memory. + // A file that could not be read says nothing about its memory, so the row + // is neither reaped as a deleted line nor counted as safe in the vault — + // nothing confirmed it there, and an unflushed flag has to stay set for + // doctor and the next index. + unreadable := map[int64]bool{} for _, uid := range edits.unreadableUIDs { var id int64 if db.QueryRow("SELECT id FROM memories WHERE uid = ? AND superseded = 0", uid).Scan(&id) == nil { - keep[id] = true + unreadable[id] = true } } + // A damaged file is reported and never acted on. Importing what survived + // would be the same silent deletion this guard exists to prevent, and the + // user needs to know before they overwrite the file by writing a new memory. if len(damaged) > 0 { return imported, rescued, fmt.Errorf( "refusing to import an incomplete memory store (%s); "+ @@ -996,6 +1007,9 @@ func importInto(db *sql.DB, p *provider.Provider, embedModel, dir string, edits rows.Close() return imported, rescued, err } + if unreadable[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 @@ -1341,6 +1355,7 @@ func readKindFiles(db *sql.DB, dir string, kind Kind, e *Edits) (raw string, exi } continue } + uid := strings.TrimSuffix(en.Name(), ".md") content, rerr := os.ReadFile(filepath.Join(kindDir, en.Name())) if rerr != nil { // Unreadable, not missing — treated the same as linesElsewhere @@ -1356,14 +1371,14 @@ func readKindFiles(db *sql.DB, dir string, kind Kind, e *Edits) (raw string, exi why = pe.Err // the path is said once, vault-relative } e.Unreadable = append(e.Unreadable, rel+": "+why.Error()) - e.unreadableUIDs = append(e.unreadableUIDs, strings.TrimSuffix(en.Name(), ".md")) + e.unreadableUIDs = append(e.unreadableUIDs, uid) } // Not migrated over either: a legacy line with this uid would // be written onto the file nobody could read. - have[strings.TrimSuffix(en.Name(), ".md")] = true + have[uid] = true continue } - have[strings.TrimSuffix(en.Name(), ".md")] = true + have[uid] = true b.Write(content) if len(content) > 0 && content[len(content)-1] != '\n' { b.WriteByte('\n') From db46667f4d887a43da71130aef966c18d9f90f9e Mon Sep 17 00:00:00 2001 From: Coder8124 Date: Mon, 2 Nov 2026 15:29:06 -0800 Subject: [PATCH 6/6] BUG: An index that refuses still names the memory files it could not read --- cmd/logos/index.go | 4 +++ cmd/logos/index_unreadable_memory_test.go | 37 +++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/cmd/logos/index.go b/cmd/logos/index.go index b5c31af..cba1a6d 100644 --- a/cmd/logos/index.go +++ b/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. + printConflictCopies(w, "", nil, edits) return err } printMemoryEdits(w, edits) diff --git a/cmd/logos/index_unreadable_memory_test.go b/cmd/logos/index_unreadable_memory_test.go index 12672ed..03ef7fc 100644 --- a/cmd/logos/index_unreadable_memory_test.go +++ b/cmd/logos/index_unreadable_memory_test.go @@ -97,3 +97,40 @@ func TestAnUnreadableMemoryFileIsNotForgottenByTheNextIndex(t *testing.T) { } } } + +// 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