diff --git a/cmd/codeaf-demo-home/seed.go b/cmd/codeaf-demo-home/seed.go index 56eabea58e..78224369d9 100644 --- a/cmd/codeaf-demo-home/seed.go +++ b/cmd/codeaf-demo-home/seed.go @@ -125,7 +125,7 @@ func seedDemoHome(dir string, now time.Time) (builtHome, error) { } built.Standing = orders - memories, err := writeMemories(brain) + memories, err := writeMemories(brain, projects[firstProjectName].dir) if err != nil { return built, err } diff --git a/cmd/codeaf-demo-home/seed_memory.go b/cmd/codeaf-demo-home/seed_memory.go index bbf7e694f4..5390ae74dd 100644 --- a/cmd/codeaf-demo-home/seed_memory.go +++ b/cmd/codeaf-demo-home/seed_memory.go @@ -12,6 +12,7 @@ package main import ( "fmt" + "github.com/Agent-Field/codeaf/internal/gitidentity" "github.com/Agent-Field/codeaf/internal/store" ) @@ -73,12 +74,29 @@ var demoMemories = []demoMemory{ } // writeMemories puts every memory into the store and answers how many it wrote. -func writeMemories(brain *store.Store) (int, error) { +// +// THE PROJECT ROWS CARRY A REAL OWNER. A scope word alone would land them in +// the quarantine ([store.OwnerLegacyProject]) — the honest answer for a row +// whose project nobody proved, and the wrong one for a fixture whose project +// is sitting right there: the demo home would show its project shelf all +// quarantined rows and never a live project memory. The key is minted the way +// every session mints one, from the workspace itself, so the demo's memory is +// visible exactly where the demo's conversations run. +func writeMemories(brain *store.Store, projectDir string) (int, error) { + key, err := gitidentity.ProjectKey(projectDir) + if err != nil || key == "" { + return 0, fmt.Errorf("mint the demo project key: %w", err) + } + projectOwner := store.OwnerProject(key) written := 0 for _, held := range demoMemories { - made, err := brain.AddMemory(held.memory) + memory := held.memory + if memory.Scope == store.MemoryScopeProject { + memory.Owner = projectOwner + } + made, err := brain.AddMemory(memory) if err != nil { - return written, fmt.Errorf("remember %q: %w", held.memory.Title, err) + return written, fmt.Errorf("remember %q: %w", memory.Title, err) } // One call per count: the two counters are written together from one // confirmation after a turn, and there is no door that sets them to a diff --git a/cmd/codeaf/chatv3.go b/cmd/codeaf/chatv3.go index 00d05f2034..a0443dcfc7 100644 --- a/cmd/codeaf/chatv3.go +++ b/cmd/codeaf/chatv3.go @@ -23,6 +23,7 @@ import ( "github.com/Agent-Field/codeaf/internal/effort" "github.com/Agent-Field/codeaf/internal/enginehost" "github.com/Agent-Field/codeaf/internal/env" + "github.com/Agent-Field/codeaf/internal/gitidentity" "github.com/Agent-Field/codeaf/internal/guard" "github.com/Agent-Field/codeaf/internal/home" "github.com/Agent-Field/codeaf/internal/leave" @@ -980,6 +981,20 @@ func openV3Launch(proc *v3Process, opts v3Options) (*v3Launch, error) { // memory row is on, which is what makes "memory off makes no calls" a // fact about the wiring instead of a branch every caller has to keep. Memory: proc.Memory, + // AND THE CONVERSATION INDEX, WHICH IS NOT THE MEMORY ANY MORE. It is + // the process's search store — the same handle as Memory when memory + // is on, and its own handle of the same file when memory is off — so + // the search verb survives the memory row. The chat journal indexes + // the conversation into it either way; the memory row now decides + // only what is REMEMBERED (internal/session's memory.go). + MemoryIndex: proc.Search, + // AND THE PROJECT THIS SESSION CAN PROVE IT IS IN. The key is minted + // from the workspace's origin remote, or its canonical path when there + // is no remote, and it is what turns a `project` scope word into the + // owner a memory really belongs to. Empty (an error, or a workspace + // that could not be resolved) quarantines project writes rather than + // guessing. + MemoryProjectKey: v3ProjectKey(workspace), // AND THE SKILL SHELF, which is the line above when memory is on and a // shelf of the skill folders alone when it is off ([v3SkillShelf]): // the skills a person installed for another harness are not memory, @@ -2100,6 +2115,35 @@ func v3BuiltinApprovals() map[string]any { // made by [store.Open] itself, which is the one door every caller goes through, // and what reaches the line below is only ever a real reason: it names the file // and says what the disk said about it. +// v3SearchStore opens the conversation index with the memory row OFF. It is +// the same file the memory store is ([defaultChatDB]), opened when the memory +// store was not — and its failures are the same non-fatal shape: a person who +// wanted a conversation gets one, and the search verb answers what a missing +// index honestly answers. +func v3SearchStore(profileDir string) *store.Store { + brain, err := store.Open(defaultChatDB()) + if err != nil { + fmt.Fprintln(os.Stderr, "conversation search is off for this session: "+err.Error()) + return nil + } + return brain +} + +// v3ProjectKey mints the key this launch's workspace can prove it is. It is +// the origin remote's normalized identity when the workspace is a git +// repository with one, and the canonical path's hash when there is none — +// never a guess. An error or an empty workspace answers "", and a session +// carrying "" quarantines its project writes rather than attributing them +// (internal/gitidentity's project.go states the rule; internal/session's +// memory.go states where the empty key lands). +func v3ProjectKey(workspace string) string { + key, err := gitidentity.ProjectKey(workspace) + if err != nil || strings.TrimSpace(key) == "" { + return "" + } + return key +} + func v3Memory(profileDir string) *store.Store { if !config.MemoryEnabledAt(profileDir) { return nil @@ -2137,8 +2181,29 @@ func (s v3Brain) ChangedSince(t time.Time) (int, int, error) { return s.brain.MemoryChangedSince(t) } +// ListMemories answers the surface's scope word as the OWNERS it means. The +// person's overview reads the whole store (the snapshot above does), so an +// empty scope stays "every owner"; `project` is the word the surface spells +// for "the project rows", and it answers every project's rows — this is the +// person's own audit page, not a session's retrieval, and the session-scoped +// reads live in internal/session. A scope this build does not spell answers +// nothing rather than everything. func (s v3Brain) ListMemories(scope string, limit int) ([]store.Memory, error) { - return s.brain.ListMemories(scope, limit) + switch strings.TrimSpace(scope) { + case "": + return s.brain.ListMemories(nil, limit) + case store.MemoryScopeUser: + return s.brain.ListMemories([]string{store.OwnerUser}, limit) + case store.MemoryScopeEnv: + return s.brain.ListMemories([]string{store.OwnerMachine}, limit) + case store.MemoryScopeProject: + // EVERY PROJECT'S ROWS, INCLUDING THE QUARANTINE: this is the person's + // audit shelf, not a session's retrieval — a project-scoped listing + // that answered only the quarantine would hide every real project's + // memories the day the first one was written with an owner. + return s.brain.ListProjectMemories(limit) + } + return nil, nil } func (s v3Brain) UpdateMemory(id, title, text string, tags []string) error { diff --git a/cmd/codeaf/chatv3_layout.go b/cmd/codeaf/chatv3_layout.go index 65ac09c628..b8216025c9 100644 --- a/cmd/codeaf/chatv3_layout.go +++ b/cmd/codeaf/chatv3_layout.go @@ -415,12 +415,20 @@ func v3PointAt(cfg session.Config, place session.Place) (session.Config, error) if workspace := strings.TrimSpace(place.Workspace); workspace != "" { cfg.Workspace = workspace } + cfg.MemoryProjectKey = v3ProjectKey(cfg.Workspace) return cfg, nil } if err := prepareOwnedWorkspace(place); err != nil { return cfg, err } cfg.Workspace = place.Work() + // AND THE PROJECT KEY FOLLOWS THE WORKSPACE, because the key IS the + // workspace's provable identity: a conversation moved to another session + // folder — /new, a resume, a picker — is a conversation that may be in a + // different project, and a memory scoped to the wrong key is a memory in + // the wrong place. Recomputed here, where the workspace is settled, so + // every door through this function agrees about whose project this is. + cfg.MemoryProjectKey = v3ProjectKey(cfg.Workspace) return cfg, nil } diff --git a/cmd/codeaf/chatv3_process.go b/cmd/codeaf/chatv3_process.go index f8b86d940e..d25c469d7f 100644 --- a/cmd/codeaf/chatv3_process.go +++ b/cmd/codeaf/chatv3_process.go @@ -30,6 +30,7 @@ import ( "context" "errors" "fmt" + "github.com/Agent-Field/codeaf/internal/gitidentity" "os" "path/filepath" "strings" @@ -92,6 +93,17 @@ type v3Process struct { // answer to give. Each conversation still gets its own memory pass and its // own context, which is per-agent already. Memory *store.Store + // Search is the store the CONVERSATION INDEX lives in — the same file as + // Memory when memory is on, and the same file opened alone when it is off. + // THAT IS THE WHOLE DECOUPLING: the memory row turns off what the session + // REMEMBERS, never what the next conversation can SEARCH. It used to be + // one field and one decision, so turning memory off also closed the + // conversation index and the search verb with it — a person who turned + // memory off lost a feature they never asked to lose. + // + // Search is legitimately PROFILE-scoped, for Memory's reason (one handle + // per file per process; two pools on one file arbitrate through a timeout). + Search *store.Store // Skills is the skill shelf every conversation this process opens reads: // the Memory store itself when memory is on, and otherwise a store of its // own that holds nothing but the skills the folders on disk hold @@ -256,6 +268,16 @@ func openV3ProcessWith(door string, askKey bool) (*v3Process, error) { wirePoolIndex(settings.ProfileDir) shelf := newV3ModelShelf(models, discovery) shelf.setSources(settings.Sources) + memory := v3Memory(settings.ProfileDir) + // THE SEARCH STORE IS THE MEMORY STORE WHEN MEMORY IS ON — one handle, one + // file — and its own handle of the same file when memory is off. Never two + // handles at once: [v3Process.Memory]'s law is about one file, one pool. + var search *store.Store + if memory != nil { + search = memory + } else { + search = v3SearchStore(settings.ProfileDir) + } process := &v3Process{ Settings: settings, ProfileDir: settings.ProfileDir, @@ -265,12 +287,20 @@ func openV3ProcessWith(door string, askKey bool) (*v3Process, error) { processStop: processStop, Shelf: shelf, Harnesses: subharness.Default(), - Memory: v3Memory(settings.ProfileDir), + Memory: memory, + Search: search, Artifacts: artifactsIndexPath(), Conns: v3Connect(settings.ProfileDir), LaunchDir: launchDir, } process.Skills, process.skillsDir = v3SkillShelf(process.Memory) + // AND THE QUARANTINE IS GIVEN ITS OWNERS BACK, once per process, only when + // there is something to give: rows the migration could not attribute are + // re-homed to the project their source session provably ran in. This is + // the "recover authoritative source ownership" half of the migration — the + // store quarantined rather than guessed, and the disk (where a session's + // workspace is a fact) is what does the proving. + v3RehomeLegacyMemories(memory) process.creditWatcher = newV3CreditWatcher(process) process.startPlaceSweep() return process, nil @@ -650,6 +680,14 @@ func (p *v3Process) closeAll() { if p.Memory != nil { _ = p.Memory.Close() } + // THE SEARCH HANDLE CLOSES ONLY WHEN IT IS NOT THE MEMORY'S. When memory + // is on the two fields are one handle and the close above already took + // it; when memory is off the search store was opened on its own, and a + // handle left open here is a WAL that never checkpoints and a file the + // next process reads mid-write. + if p.Search != nil && p.Search != p.Memory { + _ = p.Search.Close() + } // The memory-off shelf goes with the process that built it: it was only // ever a reading of the skill folders, and the next launch reads them // again. @@ -948,3 +986,44 @@ func (p *v3Process) liveSettings(base config.Config) func() config.Config { return live } } + +// v3RehomeLegacyMemories moves every quarantined memory whose source session +// can be proven to a workspace into that workspace's project owner, and leaves +// the rest in quarantine — a row whose session cannot be found has an owner +// nobody can name, and attributing it anyway is the guess the migration +// refused to make. +// +// It runs at process open, after the store is open and before the first +// conversation builds, and only when the quarantine holds rows: one pass, a +// bounded read of the places root, and nothing when the migration's work was +// already done. +func v3RehomeLegacyMemories(brain *store.Store) { + if brain == nil { + return + } + rows, err := brain.QuarantinedMemories(500) + if err != nil || len(rows) == 0 { + return + } + bySession := session.SessionWorkspaces() + owners := map[string][]string{} + for _, row := range rows { + workspace := bySession[row.SourceSession] + if workspace == "" { + continue + } + key, err := gitidentity.ProjectKey(workspace) + if err != nil || strings.TrimSpace(key) == "" { + continue + } + owner := store.OwnerProject(key) + owners[owner] = append(owners[owner], row.ID) + } + for owner, ids := range owners { + if _, err := brain.RehomeMemories(ids, owner); err != nil { + // A failed re-home is not fatal and not silent: the rows stay in + // quarantine, and the complaint names where. + fmt.Fprintln(os.Stderr, "memory: could not re-home quarantined rows: "+err.Error()) + } + } +} diff --git a/cmd/codeaf/engine.go b/cmd/codeaf/engine.go index 211a7bd060..5034cd409f 100644 --- a/cmd/codeaf/engine.go +++ b/cmd/codeaf/engine.go @@ -1098,10 +1098,13 @@ func bootEngine(hello remote.Hello, workspaceFlag, sessionFlag string) (*remote. return remote.LedgerReading{Lines: lines, Held: held} }, Search: func(terms string, limit int) ([]store.ConversationHit, error) { - if proc.Memory == nil { - return nil, errors.New("memory is off") + // THE SEARCH STORE, NOT THE MEMORY STORE. Memory off used to take + // this down with it; the process keeps the index open either way + // ([v3Process.Search]). + if proc.Search == nil { + return nil, errors.New("conversation search is unavailable") } - return proc.Memory.SearchConversations(terms, limit) + return proc.Search.SearchConversations(terms, limit) }, Memory: v3MemorySeam(proc.Memory), Archive: session.SetArchived, diff --git a/docs/changes/unreleased/1745-memory-owners-one-door-visible-failures.md b/docs/changes/unreleased/1745-memory-owners-one-door-visible-failures.md new file mode 100644 index 0000000000..ed87e3c163 --- /dev/null +++ b/docs/changes/unreleased/1745-memory-owners-one-door-visible-failures.md @@ -0,0 +1,24 @@ +--- +kind: added +title: memories belong to an owner, every write walks one door, and failure is visible +pr: 1745 +surface: [chat, engine, docs] +invalidates: + - "The memory `scope` column was a label, not an owner: the router's shortlist, the dedup search and a forget query were unfiltered, so a project's memory could surface — and be updated — in an unrelated project. Every memory now carries an `owner` (`user`, `machine`, `project:`), every read that can put a memory in front of a model filters on it, and the reads without one are refused rather than allowed to widen." + - "A project's identity was the word `project` and nothing else. It is now proved from the workspace: the git origin's normalized URL (an SSH clone and an HTTPS clone of one repository are one project), or the canonical path when there is no remote. A memory whose project cannot be proved is quarantined, never re-attributed to the person at large; rows the migration can prove — through their source session's own workspace record — are re-homed to that project." + - "The routed `remember…` command wrote through AddMemory directly, the one door with no duplicate check, so the same preference said three ways through three mouths could be three rows. Every mouth — /remember, the remember tool, the routed command, the post-turn extraction and the legacy import — now walks one door with one dedup rule, and both outcomes (kept, skipped) are journaled." + - "Memory writes failed silently: an extraction the settle could not finish, a store refusal, an import that stopped halfway — nothing anywhere recorded it. Failures are now journaled (`memory_write_failed`, with the door and the reason), said once per five-minute window as a dim line, and fall back to `v3/memory-failures.log` when the store itself is what failed." + - "The legacy memory.md import renamed the file whether or not the rows landed, and ignored the scanner's own error. The rename now happens only after every row landed; a scan error or a failed row leaves the file in place and retries later, and the retry is idempotent — kept lines are skipped, never duplicated." + - "An explicit save needed the small memory model: when the decider could not answer, /remember lost the words. The decider's failure is journaled and the save still happens through the store's own dedup door, which needs no model." + - "Turning memory off also closed the conversation index, so `search_conversations` disappeared with it — a feature nobody asked to lose. The conversation index is now its own handle on the same file: the memory row decides what is REMEMBERED, never what is SEARCHABLE, and a memory-off conversation still indexes itself and still carries the search verb." + - "The FTS5 index was assumed. It is now proved by a probe at open: a build whose SQLite has no FTS5 opens with memory intact and only the lexical tier quiet, with no install step and no fallback data store." + - "The memory listing could not page: a caller asked for the first N and stopped. The store has keyset pagination (`MemoryPage`) on the same newest-touched ordering, with a cursor that survives writes landing between pages." + - "Sync was promised by the schema and delivered by nothing. The store now exports memory events after a cursor (`ExportMemoryEvents`) and folds a batch idempotently (`ApplyMemoryEvents`) with a receiver's owner policy: adds re-deliver as skips, tombstones never resurrect, first-applied wins on concurrent supersession, and refused owners are skipped, never widened. No transport ships with this — PR #1738's pairing carries it later." +--- + +The memory architecture review (docs/memory-architecture.md) found four +structural defects in the pipeline — scope leak, split write doors, silent +failure, lossy import — and this change lands the substrate that closes them. +The architecture document is updated in the same change to say what shipped +and what is deliberately deferred (embedding vectors, team owners, the sync +transport). diff --git a/internal/gitidentity/project.go b/internal/gitidentity/project.go new file mode 100644 index 0000000000..3b9c8fff31 --- /dev/null +++ b/internal/gitidentity/project.go @@ -0,0 +1,122 @@ +package gitidentity + +// A memory's project owner is keyed on WHOSE repository the workspace is, and +// only falls back to WHERE it is. The key must survive what a workspace +// survives — a folder move, a checkout on another machine — so it is hashed +// from the git remote URL when one exists, and from the canonical path only +// when there is no remote to name. +// +// THIS IS AN IDENTITY, NOT A PATH. `project:` rows outlive renames, +// clones and moves; a path hash would have turned every `mv` of a repository +// into amnesia. What it does NOT survive yet: renaming a repository that has +// no remote, whose only identity is its path — a limitation, written into the +// architecture document as open decision 4, and the honest answer for a store +// whose rows have no alias table yet. + +import ( + "crypto/sha256" + "encoding/hex" + "os/exec" + "path/filepath" + "strings" +) + +// ProjectKey answers the stable project key of a workspace: the normalized +// origin remote's hash when the workspace is a git repository with an origin, +// and the canonical path's hash otherwise. The second answer is also the +// error-free answer for a workspace that is not a repository at all. +// +// The key is opaque by design and the same length either way — 16 bytes of +// SHA-256 — so nothing that reads an owner string can tell a remote-keyed +// project from a path-keyed one and start treating them differently. +func ProjectKey(workspace string) (string, error) { + root := strings.TrimSpace(workspace) + if root == "" { + return "", nil + } + if remote := originRemote(root); remote != "" { + if normalized := NormalizeRemoteURL(remote); normalized != "" { + return hashKey(normalized), nil + } + } + absolute, err := filepath.Abs(root) + if err != nil { + return "", nil + } + return hashKey(absolute), nil +} + +// hashKey is the one way a project identity is minted. +func hashKey(value string) string { + sum := sha256.Sum256([]byte(value)) + return hex.EncodeToString(sum[:16]) +} + +// originRemote answers the workspace's origin URL, or "". Git's own answer is +// the authority; a failed read is "no remote", never a failure — the path +// fallback is the designed answer for a workspace without one. +func originRemote(root string) string { + out, err := exec.Command("git", "-C", root, "remote", "get-url", "origin").Output() + if err != nil { + return "" + } + return strings.TrimSpace(string(out)) +} + +// NormalizeRemoteURL folds the spellings of one repository down to one name. +// These are the same repository and must key the same owner: +// +// git@github.com:owner/repo.git +// ssh://git@github.com/owner/repo +// https://github.com/owner/repo +// http://github.com/owner/repo/ +// github.com/owner/repo +// +// The scheme and the credentials are dropped, a trailing .git is dropped, the +// host is lower-cased (GitHub's own rule, and the only host this +// normalization claims to speak for), and the result is scheme-less. Two +// checkouts — one cloned over SSH on one machine, one over HTTPS on another — +// are one project to the memory, which is the whole point. +func NormalizeRemoteURL(raw string) string { + value := strings.TrimSpace(raw) + if value == "" { + return "" + } + // scp-like spelling first, before anything eats the colon: git@host:owner/repo + if at := strings.Index(value, "@"); at >= 0 && strings.Contains(value[at:], ":") && !strings.Contains(value[:at], "://") { + host := value[at+1 : strings.Index(value[at:], ":")+at] + value = host + "/" + value[strings.Index(value[at:], ":")+at+1:] + } + value = strings.TrimPrefix(value, "ssh://") + value = strings.TrimPrefix(value, "git://") + value = strings.TrimPrefix(value, "https://") + value = strings.TrimPrefix(value, "http://") + value = strings.TrimPrefix(value, "git+ssh://") + value = strings.TrimPrefix(value, "git+https://") + // A port on a host spelling: github.com:22/owner/repo + if colon := strings.Index(value, ":"); colon > 0 { + port := value[colon+1:] + digits := 0 + for digits < len(port) && port[digits] >= '0' && port[digits] <= '9' { + digits++ + } + if digits > 0 && digits < len(port) && port[digits] == '/' { + value = value[:colon] + port[digits:] + } + } + value = strings.TrimSuffix(value, ".git") + // A userinfo on a URL spelling: https://git@github.com/owner/repo + if at := strings.Index(value, "@"); at >= 0 && !strings.Contains(value[:at], "/") { + value = value[at+1:] + } + value = strings.TrimSuffix(value, ".git") + value = strings.TrimSuffix(value, "/") + // The host is lower-cased; the path is not — case-sensitive hosts exist, + // and a normalization that folds their case would collide two projects. + if slash := strings.Index(value, "/"); slash > 0 { + value = strings.ToLower(value[:slash]) + value[slash:] + } else { + value = strings.ToLower(value) + } + return value +} diff --git a/internal/gitidentity/project_test.go b/internal/gitidentity/project_test.go new file mode 100644 index 0000000000..ecea8772e9 --- /dev/null +++ b/internal/gitidentity/project_test.go @@ -0,0 +1,32 @@ +package gitidentity + +import "testing" + +// The normalizer's whole claim: every spelling of ONE repository folds to ONE +// name, so a project's memories are one project's whatever road it was cloned +// by. The path after the host keeps its case — case-sensitive hosts exist, +// and folding theirs would collide two projects. +func TestNormalizeRemoteURLFoldsTheSpellingsOfOneRepository(t *testing.T) { + same := []string{ + "git@github.com:Agent-Field/codeaf.git", + "ssh://git@github.com/Agent-Field/codeaf", + "https://github.com/Agent-Field/codeaf.git", + "http://github.com/Agent-Field/codeaf/", + "https://git@github.com/Agent-Field/codeaf", + "ssh://git@github.com:22/Agent-Field/codeaf", + } + want := "github.com/Agent-Field/codeaf" + for _, raw := range same { + if got := NormalizeRemoteURL(raw); got != want { + t.Errorf("%q -> %q, want %q", raw, got, want) + } + } + // A different host is a different name, lower-cased as the host's own rule + // usually spells it. + if got := NormalizeRemoteURL("git@gitlab.company.com:grp/proj.git"); got != "gitlab.company.com/grp/proj" { + t.Errorf("gitlab -> %q", got) + } + if got := NormalizeRemoteURL(""); got != "" { + t.Errorf("empty -> %q, want empty", got) + } +} diff --git a/internal/manual/chat/what-i-remember.md b/internal/manual/chat/what-i-remember.md index af8d73ba4a..dafab666b7 100644 --- a/internal/manual/chat/what-i-remember.md +++ b/internal/manual/chat/what-i-remember.md @@ -30,6 +30,13 @@ below is made, and the background tidy never runs. With it off, `/remember`, memory is off for this session · turn it on under /settings ``` +**Turning memory off does not turn conversation search off.** Everything said +in this machine's conversations stays indexed and `search_conversations` still +works — searching what was said is reading your own history, not remembering +about you, and the memory row takes the second away, not the first. The +`remember` verb is off the belt entirely, so the model cannot try to save +anything. + Skills are not memory, and turning memory off keeps them: the skills in your Claude Code, Codex and other skill folders still reach the conversation, carried with a message that suits them and attached by `/skill` @@ -512,6 +519,12 @@ all — which is most passes. The line arrives in whichever conversation you mos recently touched, if any is open; on a machine with no window open there is no line, and the change is simply there the next time you look at `/memory`. +If a tidy write fails, it reports an error rather than recording a successful +no-op. Successful changes from that pass remain, but its completion watermark +is not advanced, so the failed work stays eligible for retry. Write failures +are also journaled when the store can accept the failure record. Quarantined +rows are excluded even if a project key is accidentally set to `legacy`. + **When it runs.** It rides the same 5-minute background pass that checks everything standing, and three things have to be true at once: memory is **on**, **nobody has said anything anywhere for fifteen minutes**, and the last tidy was @@ -520,8 +533,10 @@ must have changed since the last one — one new line has already been settled against its neighbours on the turn that wrote it, so there would be nothing to merge it with. -**What it may do.** It reads the fifty most recently touched lines, grouped by -how far each one's truth reaches, and answers with at most **eight** changes: +**What it may do.** It reads the fifty most recently touched lines — only yours and +this machine's, never a project you are not currently in and never what is in +quarantine — grouped by how far each one's truth reaches, and answers with at most +**eight** changes: - **merge** two lines that say the same thing into one clearer line, keeping every fact both of them carried; @@ -582,7 +597,10 @@ true about you everywhere (the default), `project` for something true only in this project, `env` for something true only on this machine. All three go through the same settling step, so saying it twice refines one -memory rather than making a second. +memory rather than making a second — **and the settling step no longer needs +the model to be up**: if the small memory model cannot answer, the line is +still kept through the store's own duplicate check, and the failed settle is +recorded in the journal rather than lost. ## Forgetting something @@ -652,15 +670,38 @@ pair uses it. ## Where is it kept, and does a task see it? It is kept in `~/.codeaf/graph.db`, which is per person rather than per -conversation or per project — so something remembered in one repository is -remembered in the next. `CODEAF_HOME` moves it with everything else codeaf -keeps. The same file holds every message of every conversation, which is what +conversation — but **not** one flat pool: what each conversation is SHOWN +depends on who the memory belongs to. + +- A memory kept as **you** (the default) is yours everywhere — every project, + every conversation. +- A memory kept as **this project** belongs to that project and is shown only + in conversations running in it. Two repositories can hold genuinely + different truths about the same words, and neither one's memory leaks into + the other's conversation: the router's shortlist, a `/memories ` and + a `/forget ` in this project can never answer with another project's + memory. +- A memory kept as **this machine** stays on this machine. + +Which project a conversation is in is proved, not guessed: the project's git +`origin` URL (folded so that an SSH clone and an HTTPS clone of the same +repository are the same project), or the folder itself when there is no +remote. A memory whose project cannot be proved — rows kept by older builds +that recorded only the word `project` — is **quarantined** rather than +re-attributed: it is never shown to any conversation automatically, and the +ones whose source conversation can be traced to a folder are moved back to +that project's own memories. The rest wait on the memory place under the +project shelf, marked `legacy-project`, for you to keep, re-word or drop. + +`CODEAF_HOME` moves the whole file with everything else codeaf keeps. The +same file holds every message of every conversation, which is what `search_conversations` searches; the transcripts themselves stay in each conversation's own folder. **A task gets the same treatment as a message.** When work is handed off to a -task, the router is asked once against that task's brief, and whatever it names -is put at the top of the task's own instructions. The task never writes memories +task, the router is asked once against that task's brief, and whatever it +names is put at the top of the task's own instructions — scoped to the same +owners the conversation that started it can see. The task never writes memories of its own: a family of eight tasks would otherwise be eight writers on one brain, all blind to each other. @@ -795,6 +836,31 @@ imported 12 memories from memory.md After that the file is gone from codeaf's view and the store is the only memory. +**The rename is the last thing that happens.** If the file could not be read in +full, or a line could not be kept, the file stays exactly where it is, the +failure is written into the store's journal, and the import is tried again on a +later turn. A retry never makes duplicates: a line the first attempt already +kept is recognised as kept and skipped, so a file that half-imported once +finishes cleanly the second time. + +## When a memory write fails + +Memory writes used to fail silently — an extraction the small memory model +could not settle, a line the store refused, an import that stopped halfway. +Now every failure lands in the store's journal (`memory_write_failed`, with +the door that failed and the reason), and the conversation says one dim line +about it, at most once every five minutes however many failures pile up: + +``` +couldn't settle a memory just now · the journal has the reason +``` + +If the store itself is what failed — a disk out of room, a file that will not +take writes — the reason goes to `~/.codeaf/v3/memory-failures.log` instead, +one line per failure, so there is always somewhere the answer lives. Nothing +is retried automatically except the legacy import above; the next turn's +extraction simply tries again the normal way. + ## It used to say preparing saved context — why is that gone, and does the lookup slow my answer down? It is gone because **the lookup is not a wait any more**, and a status word for diff --git a/internal/remote/memory_engine_regression_test.go b/internal/remote/memory_engine_regression_test.go index 986b65265b..d276872a3e 100644 --- a/internal/remote/memory_engine_regression_test.go +++ b/internal/remote/memory_engine_regression_test.go @@ -158,6 +158,8 @@ func TestDefaultEngineMemoryOffIsNotAnEmptyEnabledStore(t *testing.T) { // The test uses the store's own rows for the place, the same translation the // executable's v3Brain supplies, so command writes and page reads share a brain. +// ListMemories spells the scope words the wire carries, the way the executable's +// door does. type engineMemoryRows struct{ *store.Store } func (m engineMemoryRows) Snapshot(limit int) (store.MemoryShelves, error) { @@ -168,6 +170,22 @@ func (m engineMemoryRows) ChangedSince(at time.Time) (int, int, error) { return m.MemoryChangedSince(at) } +func (m engineMemoryRows) ListMemories(scope string, limit int) ([]store.Memory, error) { + switch strings.TrimSpace(scope) { + case "": + return m.Store.ListMemories(nil, limit) + case store.MemoryScopeUser: + return m.Store.ListMemories([]string{store.OwnerUser}, limit) + case store.MemoryScopeEnv: + return m.Store.ListMemories([]string{store.OwnerMachine}, limit) + case store.MemoryScopeProject: + // THE SAME ANSWER THE EXECUTABLE'S DOOR GIVES: every project's rows, + // the quarantine included ([store.Store.ListProjectMemories]). + return m.Store.ListProjectMemories(limit) + } + return nil, nil +} + // The recorder leaves the real memory implementation intact and only exposes // its completion, so the test never polls a store racing its writer. type memorySaveRecorder struct { diff --git a/internal/session/agent.go b/internal/session/agent.go index ce87bcab47..74d691df4f 100644 --- a/internal/session/agent.go +++ b/internal/session/agent.go @@ -360,7 +360,11 @@ func newAgent(config Config, client Completer) (*Agent, error) { // And it takes the DROPPINGS home for the registry's reason: the only thing // the journal does with a Place is spill an over-long message's bytes through // [writeStub], which is a dropping like any other (landing.go). - agent.chatlog = newChatJournal(config.Memory, agent.threadID(), config.Workspace, config.droppingsPlace()) + // THE JOURNAL INDEXES INTO THE CONVERSATION INDEX, which is the memory + // store for every pre-split caller and its own store where a door opened + // one — memory off keeps the conversation searchable (tools_conversations.go + // states the order). + agent.chatlog = newChatJournal(config.conversationIndexStore(), agent.threadID(), config.Workspace, config.droppingsPlace()) // And the state card is held before any turn has run, so that the note the // first request carries already has it: a resumed conversation's card is what // it knew yesterday, and a model that had to wait for the first post-turn diff --git a/internal/session/memory.go b/internal/session/memory.go index dad811fd6d..3c620e5f50 100644 --- a/internal/session/memory.go +++ b/internal/session/memory.go @@ -63,6 +63,7 @@ import ( "github.com/Agent-Field/agentfield/sdk/go/ai" "github.com/Agent-Field/codeaf/internal/exec/bare" + "github.com/Agent-Field/codeaf/internal/home" "github.com/Agent-Field/codeaf/internal/lane" "github.com/Agent-Field/codeaf/internal/provider" "github.com/Agent-Field/codeaf/internal/reflex" @@ -123,6 +124,11 @@ type memoryBrain struct { // The rename on disk is the durable answer; this is what keeps a session // from stat-ing the same absent file every turn. imported bool + // failureSaid is when the last "couldn't settle that" line went out. A + // failure is said once per window, not once per failure: an outage that + // fails forty extractions in a row would otherwise be forty lines about + // the same thing on a screen somebody is trying to work in. + failureSaid time.Time } func newMemoryBrain(s *store.Store) *memoryBrain { @@ -148,6 +154,47 @@ func (a *Agent) memorySourceSession() string { return a.sessionID() } +// ── who the memory belongs to ─────────────────────────────────────────────── + +// ownerForScope answers the OWNER a memory written under a scope word lands +// in. This is the caller propagation, in one function: the DOOR names this +// session's project ([Config.MemoryProjectKey]), the session turns a scope +// word into the owner that scope really means, and nothing anywhere writes a +// memory whose owner was not spelled by this chain. +// +// THE EMPTY PROJECT KEY IS THE QUARANTINE'S WHOLE REASON. A session whose door +// could not prove which project it is in maps a `project` write to the +// quarantine owner — never to the person at large. A quarantined row is never +// injected anywhere, so the honest cost of an unprovable project is that its +// memory waits in the memory place for the person to re-home it. +func (a *Agent) ownerForScope(scope string) string { + switch strings.ToLower(strings.TrimSpace(scope)) { + case store.MemoryScopeEnv: + return store.OwnerMachine + case store.MemoryScopeProject: + if key := strings.TrimSpace(a.config.MemoryProjectKey); key != "" { + return store.OwnerProject(key) + } + return store.OwnerLegacyProject + default: + return store.OwnerUser + } +} + +// memoryOwners is what this session may SEE: the person, this machine, and +// this project. Every read that can put a memory in front of a model — the +// router's shortlist, the dedup search, a forget match, a person's query — +// names exactly this list, and the store refuses to answer without it. A task +// worker inherits the same config, so a node's reading is scoped the same way +// its parent's was. +func (a *Agent) memoryOwners() []string { + owners := []string{store.OwnerUser, store.OwnerMachine} + if key := strings.TrimSpace(a.config.MemoryProjectKey); key != "" { + owners = append(owners, store.OwnerProject(key)) + } + return owners +} + // reflexClient is this session's own client pinned to the reflex model, or nil // when the ladder cannot name one. // @@ -676,7 +723,7 @@ func (a *Agent) routedMemory(ctx context.Context, cue string, say func(string), // near-synonymous preferences is nothing but hard distractors, and rejecting // them is the one job arithmetic cannot do. Showing it the haystack is what // stops. - candidates, err := a.memory.store.MemoryCandidates(cue, store.MemoryCandidatesDefault) + candidates, err := a.memory.store.MemoryCandidates(a.memoryOwners(), cue, store.MemoryCandidatesDefault) if err != nil || len(candidates) == 0 { // AN EMPTY SHORTLIST IS NOT A CALL, and with two arithmetic rankings // under it an empty one means an empty store. There is nothing to route @@ -702,7 +749,7 @@ func (a *Agent) routedMemory(ctx context.Context, cue string, say func(string), if len(routed.Inject) == 0 { return "" } - memories, err := a.memory.store.GetMemories(routed.Inject) + memories, err := a.memory.store.GetMemories(a.memoryOwners(), routed.Inject) if err != nil || len(memories) == 0 { return "" } @@ -720,6 +767,13 @@ func (a *Agent) routedMemory(ctx context.Context, cue string, say func(string), // It is answered in one dim line and nothing else. The person gave an // instruction and it either happened or it did not; a card, an event kind or a // paragraph would all be this surface making a ceremony out of a note. +// +// THE REMEMBER HALF WALKS THE ONE WRITE DOOR. It used to call AddMemory +// directly — the one door that skipped dedup — so a person who said "remember +// I prefer tabs" in three sessions could hold three rows the other mouths +// would have refined into one. It now goes through the same +// settle-and-apply path /remember and the remember tool walk, and the line it +// says is the title that actually landed. func (a *Agent) runMemoryCommand(say func(string), cmd reflex.Cmd) { if strings.TrimSpace(cmd.Arg) == "" { return @@ -729,20 +783,16 @@ func (a *Agent) runMemoryCommand(say func(string), cmd reflex.Cmd) { } switch cmd.Name { case "remember": - memory, err := a.memory.store.AddMemory(store.Memory{ - Type: memoryTypeOf(cmd.Arg), - Scope: store.MemoryScopeUser, - Title: memoryTitleFrom(cmd.Arg), - Text: cmd.Arg, - SourceSession: a.memorySourceSession(), - }) + title, err := a.writeRemembered(cmd.Arg, "") if err != nil { + a.journalMemoryFailure("routed-remember", err) return } - say("remembered · " + memory.Title) + say("remembered · " + title) case "forget": title, err := a.forgetMatching(cmd.Arg) if err != nil { + a.journalMemoryFailure("routed-forget", err) return } if title == "" { @@ -886,10 +936,15 @@ func (a *Agent) learnFromTurn(userMsg, assistantMsg string) { } found, err := reflex.Extract(ctx, client, userMsg, assistantMsg, injected) if err != nil { - // AND NOTHING IS COUNTED. The accounting below is the extractor's - // answer; a provider outage is not evidence that a memory failed to - // help, and recording it as one would let somebody else's bad - // afternoon push a good line down the store's ranking. + // AND NOTHING IS COUNTED — but the failure is no longer SILENT. + // The accounting below is the extractor's answer; a provider + // outage is not evidence that a memory failed to help, and + // recording it as one would let somebody else's bad afternoon push + // a good line down the store's ranking. What changed is that the + // outage is journaled and, once per window, said: a memory system + // that quietly stops learning is the failure this file exists to + // make visible. + a.journalMemoryFailure("extract", err) return } a.recordMemoryOutcome(injected, found.Used) @@ -906,7 +961,14 @@ func (a *Agent) learnFromTurn(userMsg, assistantMsg string) { if found.Mem == 0 { return } - _, _ = a.applyCandidate(ctx, client, found) + if _, err := a.applyCandidate(ctx, client, found); err != nil { + // THE SETTLE FAILED, AND THAT IS SAID — once per window — with the + // reason in the journal. It used to be `_, _ =`: a memory the + // session noticed could fail to land and nothing anywhere would + // know, which is the third of the four defects this file was + // rewritten for. + a.journalMemoryFailure("settle", err) + } }() } @@ -950,6 +1012,74 @@ func (a *Agent) recordMemoryOutcome(injected []reflex.Stub, used []string) { _, _ = a.memory.store.SnapshotMemoryRanking(fixDecayInterval) } +// memoryFailureWindow is how long one "couldn't settle that" line buys. Five +// minutes is the smallest interval that is honest — long enough that an +// afternoon outage is a handful of lines, short enough that a single unlucky +// turn is not followed by silence. +const memoryFailureWindow = 5 * time.Minute + +// journalMemoryFailure is what a failed memory write becomes: an event in the +// store's journal when the store can take one, a line in a fallback file when +// the store itself is what failed, and a dim line to the person at most once +// per window. Errors used to vanish at every one of these doors; nothing +// here vanishes now, and nothing here can make a failed turn a failed day. +// +// THE FALLBACK FILE IS THE DB-IS-THE-PROBLEM ANSWER. Journaling the failure +// needs the journal, and a disk that refused the last write will refuse this +// one too — so when the journal write fails, the reason goes to +// `v3/memory-failures.log` beside the state root instead, appended through the +// one door a log append has. If even that fails, the failure is dropped ON +// PURPOSE: there is nothing left to record it in, and a session that cannot +// write anywhere must not start failing turns about it. +func (a *Agent) journalMemoryFailure(via string, err error) { + if err == nil { + return + } + reason := err.Error() + if storeErr := a.memory.store.JournalMemoryFailure(via, err); storeErr != nil { + // The journal could not take it. The file is the fallback. + if path := memoryFailureLogPath(); path != "" { + appendMemoryFailureLog(path, via, reason, storeErr.Error()) + } else { + log.Printf("session: a memory write failed and could not be journaled (via %s): %v", via, err) + } + } + // AND THE PERSON IS TOLD, ONCE PER WINDOW. A dim line in the stream the + // memory lines already ride — held for the next turn exactly as every + // other memory line is when no stream is live. + a.memory.mu.Lock() + window := time.Since(a.memory.failureSaid) >= memoryFailureWindow + if window { + a.memory.failureSaid = time.Now() + } + a.memory.mu.Unlock() + if window { + a.sayMemory("couldn't settle a memory just now · the journal has the reason") + } +} + +// memoryFailureLogPath is where the fallback carries a failure the store +// could not: v3/memory-failures.log beside the state root, moved by +// CODEAF_HOME with everything else. Empty never happens in practice; the +// answer is "" and the caller logs to stderr instead. +func memoryFailureLogPath() string { + return home.Join("v3", "memory-failures.log") +} + +// appendMemoryFailureLog appends one line, once, best effort. It is a log for +// a person debugging a disk, not a second journal: no rotation, no reader in +// the product, one line per failure with the store's own complaint beside it. +func appendMemoryFailureLog(path, via, reason, storeErr string) { + file, err := os.OpenFile(path, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o600) + if err != nil { + return + } + defer file.Close() + stamp := time.Now().UTC().Format(time.RFC3339) + line := stamp + " via=" + via + " error=" + reason + " journal_error=" + storeErr + "\n" + _, _ = file.WriteString(line) +} + // startMemoryJob registers one background memory pass, and refuses once the // session is closing. It is the [jobRegistry]'s bargain in miniature: the work // is tracked, so Close can wait for it, and cancelled, so Close does not wait @@ -974,27 +1104,31 @@ func (a *Agent) startMemoryJobLocked() bool { // answer. It is the ONE write path: the post-turn pass and the `remember` tool // both come through here, so a memory written by the model and a memory the // session noticed cannot be deduplicated by two different rules. +// +// THE ADD WALKS THE STORE'S OWN DEDUP DOOR ([store.Store.Write]), which +// searches only the candidate's owner: two projects may hold genuinely +// different truths about the same words, and neither write swallows the +// other. func (a *Agent) applyCandidate(ctx context.Context, client reflex.Completer, candidate reflex.ExtractResult) (store.Memory, error) { if strings.TrimSpace(candidate.Text) == "" { return store.Memory{}, errors.New("session: a memory with no text says nothing") } - fresh := store.Memory{ - Type: candidate.Type, - Scope: candidate.Scope, - Title: strings.TrimSpace(candidate.Title), - Text: candidate.Text, - Tags: candidate.Tags, - SourceSession: a.memorySourceSession(), - } - if fresh.Title == "" { - fresh.Title = memoryTitleFrom(candidate.Text) - } - neighbors, err := a.memory.store.SearchMemories(candidate.Text, memoryNeighbors) - if err != nil || len(neighbors) == 0 { + fresh := a.memoryDraft(candidate) + owners := a.memoryOwners() + neighbors, err := a.memory.store.SearchMemories(owners, candidate.Text, memoryNeighbors) + if err != nil { + // A SEARCH THAT FAILED IS NOT "NOTHING NEARBY". The old read treated + // both the same and added on top of whatever it could not see; a + // failed search is refused, journaled, and nothing is written — the + // next turn's extraction can settle it again, which is cheaper than a + // duplicate the door never got to catch. + return store.Memory{}, fmt.Errorf("the dedup search failed: %w", err) + } + if len(neighbors) == 0 { // NOTHING NEAR IT IS NOT A QUESTION. A store with no opinion about this // subject has nothing for a decider to weigh, and asking anyway would be // a call whose answer is known. - return a.memory.store.AddMemory(fresh) + return a.addThroughDoor(fresh) } near := make([]reflex.Neighbor, 0, len(neighbors)) for _, neighbor := range neighbors { @@ -1015,16 +1149,35 @@ func (a *Agent) applyCandidate(ctx context.Context, client reflex.Completer, can } switch decided.Op { case "add": - return a.memory.store.AddMemory(fresh) + return a.addThroughDoor(fresh) case "update": // A MISSING TARGET IS A SKIP. internal/reflex validates the enum and // leaves the id to the only thing that knows whether it names anything; // this is that thing, and the honest answer to "refine the memory that // is not there" is to change nothing. + // + // THE TARGET IS CHECKED AGAINST THE OWNERS THIS SESSION CAN SEE. A + // decider naming another project's id — from a stale neighbor list, a + // replayed conversation, a model's invention — changes nothing here, + // because the store's update runs under this session's owner filter and + // this check makes the skip visible before the store is asked. + // + // A STORE ERROR ON THE VISIBILITY CHECK REFUSES THE WRITE. The old guard + // treated both a miss and an error as a signal to proceed: a transient + // store failure converted the one cross-owner check into a silent pass, + // letting a model-named foreign id land. An error here is returned — the + // write is retryable through the door. if decided.TargetID == "" { return store.Memory{}, nil } - if err := a.memory.store.UpdateMemoryFromSession(decided.TargetID, fresh.Title, fresh.Text, fresh.Tags, fresh.SourceSession); err != nil { + visible, err := a.memory.store.GetMemories(owners, []string{decided.TargetID}) + if err != nil { + return store.Memory{}, fmt.Errorf("checking update target: %w", err) + } + if len(visible) == 0 { + return store.Memory{}, nil + } + if err := a.memory.store.UpdateMemoryForOwners(owners, decided.TargetID, fresh.Title, fresh.Text, fresh.Tags, fresh.SourceSession); err != nil { return store.Memory{}, err } return store.Memory{ID: decided.TargetID, Title: fresh.Title, Text: fresh.Text}, nil @@ -1034,9 +1187,15 @@ func (a *Agent) applyCandidate(ctx context.Context, client reflex.Completer, can } // The old line is read BEFORE it is retired, because the note names it // and a read afterwards would be a second query for a row this one - // already had in hand. - retired, _, _ := a.memory.store.MemoryRecord(decided.TargetID) - replacement, err := a.memory.store.SupersedeMemory(decided.TargetID, fresh) + // already had in hand — and read through the owners this session can + // see, for exactly the same reason the update's target is: a decider + // naming another project's id must not retire that project's memory. + visible, err := a.memory.store.GetMemories(owners, []string{decided.TargetID}) + if err != nil || len(visible) == 0 { + return store.Memory{}, nil + } + retired := visible[0] + replacement, err := a.memory.store.SupersedeMemoryForOwners(owners, decided.TargetID, fresh) if err != nil { return store.Memory{}, err } @@ -1046,6 +1205,108 @@ func (a *Agent) applyCandidate(ctx context.Context, client reflex.Completer, can return store.Memory{}, nil } +// memoryDraft is the row a candidate would land as, before the decider or the +// door has its say: owner resolved, title defaulted, source named. +func (a *Agent) memoryDraft(candidate reflex.ExtractResult) store.Memory { + fresh := store.Memory{ + Owner: a.ownerForScope(candidate.Scope), + Type: candidate.Type, + Title: strings.TrimSpace(candidate.Title), + Text: candidate.Text, + Tags: candidate.Tags, + SourceSession: a.memorySourceSession(), + } + if fresh.Title == "" { + fresh.Title = memoryTitleFrom(candidate.Text) + } + return fresh +} + +// addThroughDoor is the add half of the write path: the store's dedup door, +// with the draft's owner. The door answers a skip with the row that beat this +// one, which the caller reads as "already kept" — the same answer the decider's +// skip is. +func (a *Agent) addThroughDoor(fresh store.Memory) (store.Memory, error) { + result, err := a.memory.store.Write(store.WriteRequest{ + Owner: fresh.Owner, + Type: fresh.Type, + Title: fresh.Title, + Text: fresh.Text, + Tags: fresh.Tags, + SourceSession: fresh.SourceSession, + }) + if err != nil { + return store.Memory{}, err + } + if result.Outcome == store.WriteOutcomeSkipped { + // The store already holds this. The result's Memory is the row that + // beat the write; the caller sees a title either way. + return result.Memory, nil + } + return result.Memory, nil +} + +// writeRemembered is what a mouth that says "keep this" calls, and there are +// exactly four: the routed `remember`, /remember, the `remember` tool, and — +// with no scope word — the import. It settles near-duplicates the model way +// when a reflex client is in hand, and through the store's own dedup door when +// there is none; there is no third path, which is the whole point. +// +// The returned title is the one that landed — the existing row's when the +// write was skipped, the new row's when it was added. +func (a *Agent) writeRemembered(text, scope string) (string, error) { + if !a.remembers() { + return "", errors.New("this build is not remembering anything") + } + text = strings.Join(strings.Fields(text), " ") + if text == "" { + return "", errors.New("there is nothing to remember") + } + if strings.TrimSpace(scope) == "" { + scope = store.MemoryScopeUser + } + candidate := reflex.ExtractResult{ + Mem: 1, + Type: memoryTypeOf(text), + Scope: scope, + Title: memoryTitleFrom(text), + Text: text, + } + client := a.reflexClient() + if client == nil { + // NO REFLEX IS NOT NO MEMORY. A person who typed /remember said what + // they wanted kept; refusing them because a router model is unreachable + // would be losing their words to somebody else's outage. The store's + // own dedup door still runs, so a retype is still a skip. + memory, err := a.addThroughDoor(a.memoryDraft(candidate)) + if err != nil { + return "", err + } + return memory.Title, nil + } + ctx := a.memoryContext() + memory, err := a.applyCandidate(ctx, client, candidate) + if err != nil { + // THE MODEL IS NOT A PERMISSION. A decider that could not answer — an + // outage, a nonsense reply — does not lose the words a person typed; + // they go through the store's own dedup door, which needs no model. + // The failure is journaled either way, so the person can see the + // settle never happened even though the save did. + a.journalMemoryFailure("explicit-settle", err) + memory, fallbackErr := a.addThroughDoor(a.memoryDraft(candidate)) + if fallbackErr != nil { + return "", err + } + return memory.Title, nil + } + if memory.Title == "" { + // The decider skipped it: the store already holds this, which is the + // answer rather than a failure. + return candidate.Title, nil + } + return memory.Title, nil +} + // saySuperseded is the one dim line a retirement gets, and the reason it exists // is that it used to get none. // @@ -1132,51 +1393,10 @@ func (a *Agent) Remember(text string) (string, error) { } // RememberScoped is Remember with the blast radius named: something true about -// you everywhere, only inside this project, or only on this machine. +// you everywhere, only inside this project, or only on this machine. It walks +// the one write door ([Agent.writeRemembered]), which is every mouth there is. func (a *Agent) RememberScoped(text, scope string) (string, error) { - if !a.remembers() { - return "", errors.New("this build is not remembering anything") - } - text = strings.Join(strings.Fields(text), " ") - if text == "" { - return "", errors.New("there is nothing to remember") - } - if strings.TrimSpace(scope) == "" { - scope = store.MemoryScopeUser - } - candidate := reflex.ExtractResult{ - Mem: 1, - Type: memoryTypeOf(text), - Scope: scope, - Title: memoryTitleFrom(text), - Text: text, - } - client := a.reflexClient() - if client == nil { - // NO REFLEX IS NOT NO MEMORY. A person who typed /remember said what - // they wanted kept; refusing them because a router model is unreachable - // would be losing their words to somebody else's outage. - memory, err := a.memory.store.AddMemory(store.Memory{ - Type: candidate.Type, Scope: candidate.Scope, - Title: candidate.Title, Text: candidate.Text, - SourceSession: a.memorySourceSession(), - }) - if err != nil { - return "", err - } - return memory.Title, nil - } - ctx := a.memoryContext() - memory, err := a.applyCandidate(ctx, client, candidate) - if err != nil { - return "", err - } - if memory.Title == "" { - // The decider skipped it: the store already holds this, which is the - // answer rather than a failure. - return candidate.Title, nil - } - return memory.Title, nil + return a.writeRemembered(text, scope) } // Forget drops the best match for a query and answers with the title it @@ -1189,7 +1409,10 @@ func (a *Agent) Forget(query string) (string, error) { } func (a *Agent) forgetMatching(query string) (string, error) { - found, err := a.memory.store.SearchMemories(query, 1) + // THE MATCH IS SCOPED TO WHAT THIS SESSION CAN SEE. A forget query never + // answers with another project's memory: "forget the deploy line" in this + // repository forgets the deploy line this person can see here. + found, err := a.memory.store.SearchMemories(a.memoryOwners(), query, 1) if err != nil { return "", err } @@ -1213,9 +1436,9 @@ func (a *Agent) Memories(query string) ([]MemoryLine, error) { err error ) if strings.TrimSpace(query) == "" { - found, err = a.memory.store.ListMemories("", memoryListLimit) + found, err = a.memory.store.ListMemories(a.memoryOwners(), memoryListLimit) } else { - found, err = a.memory.store.SearchMemories(query, memoryListLimit) + found, err = a.memory.store.SearchMemories(a.memoryOwners(), query, memoryListLimit) } if err != nil { return nil, err @@ -1249,6 +1472,18 @@ func (a *Agent) memoryContext() context.Context { // both the durable record that this already happened and the person's copy of // what they wrote. A second run finds no memory.md and does nothing; a person // who wants it back has the file. +// +// THE RENAME IS THE LAST THING THAT HAPPENS AND ONLY WHEN EVERY ROW LANDED. +// It used to rename whether or not the rows landed — a disk error partway +// through left a person's old memory renamed out of the way and only partly in +// the store, with nothing anywhere to say so. Now a scan error or a failed row +// leaves the file where it is, journals the failure, and resets the +// once-marker so the next turn retries. +// +// THE RETRY IS IDEMPOTENT, which is what makes the reset safe: every row goes +// through the one write door, whose dedup skips a line the first attempt +// already kept. A partially imported file finishes; a fully imported one that +// failed only at the rename does not make a thousand duplicates. func (a *Agent) importMemoryFile() { if !a.remembers() { return @@ -1284,24 +1519,33 @@ func (a *Agent) importMemoryFile() { } lines = append(lines, line) } + // THE SCAN'S OWN ERROR IS READ. A file too big for its buffer, a disk that + // went away mid-read — these arrived here as a partial list that looked + // complete, and the import that followed both renamed the file out of the + // way and kept only what the scan happened to see. A scan error leaves + // everything exactly where it is. + if err := scanner.Err(); err != nil { + _ = file.Close() + a.retryImportNextTurn(fmt.Errorf("reading %s: %w", path, err)) + return + } _ = file.Close() imported := 0 for _, line := range lines { - if _, err := a.memory.store.AddMemory(store.Memory{ - Type: store.MemoryFact, - Scope: store.MemoryScopeUser, - Title: memoryTitleFrom(line), - Text: line, - SourceSession: a.memorySourceSession(), - }); err == nil { - imported++ + if _, err := a.writeRemembered(line, store.MemoryScopeUser); err != nil { + // ONE ROW THAT DID NOT LAND STOPS THE IMPORT AND LEAVES THE FILE. + // The rows that landed stay landed; the retry skips them as + // duplicates and carries on from the first row that failed. + a.retryImportNextTurn(fmt.Errorf("importing a memory.md line: %w", err)) + return } + imported++ } - // The rename happens whether or not a line landed: a file that could not be - // imported this time will not import better next time, and a session that - // re-read it every turn would be a session that never stopped trying. if err := os.Rename(path, path+".imported"); err != nil { + // Every row landed and the rename did not: the retry re-imports into a + // dedup door that skips every row, then tries the rename again. + a.retryImportNextTurn(fmt.Errorf("moving %s out of the way: %w", path, err)) return } if imported > 0 { @@ -1309,6 +1553,17 @@ func (a *Agent) importMemoryFile() { } } +// retryImportNextTurn journals an import failure, says so, and gives the +// import its next turn back — the once-marker was set optimistically at the +// door, and a failed attempt is exactly what it was protecting against +// repeating silently. +func (a *Agent) retryImportNextTurn(err error) { + a.memory.mu.Lock() + a.memory.imported = false + a.memory.mu.Unlock() + a.journalMemoryFailure("import", err) +} + // ── the small judgements ──────────────────────────────────────────────────── // memoryTypeOf is the whole heuristic, and it is deliberately two answers wide. @@ -1346,7 +1601,7 @@ func memoryTitleFrom(text string) string { const rememberDescription = "Remember one durable thing across sessions: a preference the person stated, a correction they made, a decision that will still bind tomorrow. Write it as a standing truth in one short line ('prefers tabs over spaces in Go'), not as a log of what just happened. It is settled against what is already remembered — a near-duplicate refines the existing line rather than adding a second — and the title it landed under comes back to you. Do not remember what the transcript already holds, what the repo or AGENTS.md already records, or anything that will be false tomorrow." -const rememberSchemaJSON = `{"type":"object","properties":{"text":{"type":"string","description":"The single line to remember, in plain words"},"scope":{"type":"string","enum":["user","project","env"],"description":"How far the truth reaches: the person everywhere (default), this project only, or this machine only"}},"required":["text"],"additionalProperties":false}` +const rememberSchemaJSON = `{"type":"object","properties":{"text":{"type":"string","description":"The single line to remember, in plain words"},"scope":{"type":"string","enum":["user","project","env"],"description":"How far the truth reaches: the person everywhere (default), this project only (invisible to other projects), or this machine only"}},"required":["text"],"additionalProperties":false}` // The gloss a person reads beside a memory call is the thing itself — // "remember prefers tabs over spaces" — for the reason every other tool's gloss diff --git a/internal/session/memory_close_test.go b/internal/session/memory_close_test.go index 21666b5e13..b41e9a4b86 100644 --- a/internal/session/memory_close_test.go +++ b/internal/session/memory_close_test.go @@ -132,7 +132,7 @@ func TestClosedSessionHasNoMemoryPassStillWritingTheStore(t *testing.T) { } // AND WHAT IT WROTE IS THERE, read after Close and before the // store is: the pass reached the store inside the session's life. - kept, err := brain.ListMemories("", 10) + kept, err := brain.ListMemories(nil, 10) if err != nil { t.Fatalf("list: %v", err) } diff --git a/internal/session/memory_consolidate.go b/internal/session/memory_consolidate.go index b5b025f9f3..74c68add94 100644 --- a/internal/session/memory_consolidate.go +++ b/internal/session/memory_consolidate.go @@ -183,6 +183,25 @@ const ( // one — no conversation asked for this. const consolidateSource = "memory-tidy" +// consolidateOwners answers the authorized owner partitions a background +// consolidation pass may touch. It never includes the quarantine: a pass cannot +// prove what a quarantined row belongs to, and the architecture says quarantined +// rows are never injected anywhere. The Config's project key is included only +// when set — this pass has no project context of its own, but a caller that +// provides one will have its project rows tidied as well. +// +// A nil or empty owners list means the pass does no work at all; the caller +// should refuse rather than widen. +func consolidateOwners(cfg Config) []string { + owners := []string{store.OwnerUser, store.OwnerMachine} + if key := strings.TrimSpace(cfg.MemoryProjectKey); key != "" { + if owner := store.OwnerProject(key); owner != store.OwnerLegacyProject && store.ValidOwner(owner) { + owners = append(owners, owner) + } + } + return owners +} + // ── the seam the tick fills ───────────────────────────────────────────────── // NewMemoryTidy is the seam a door fills [standing.Ticker.Tidy] with. @@ -240,29 +259,60 @@ func NewMemoryTidy(parent Config, brainPath, root string, idle standing.Idle) st if built != nil { return standing.Tidied{}, built } - return tidyPass{brain: brain, completer: client, model: model, root: root, mark: mark}.run(ctx) + // THE OWNERS A BACKGROUND PASS MAY TOUCH. A consolidate runs while + // nobody is at the machine and has no project context — it is a + // person-level and machine-level pass, and nothing else. If the Config + // carries a project key the pass may include that project explicitly; + // a pass that cannot prove what it is authorized to tidy does not tidy + // everything in reach — it refuses with no work done. + owners := consolidateOwners(parent) + return tidyPass{brain: brain, completer: client, model: model, root: root, mark: mark, owners: owners}.run(ctx) } } // tidyPass is one pass with everything it needs already resolved: the store -// open, the client built, and the watermark read. +// open, the client built, the watermark read, and the OWNERS this pass may +// touch. // -// It is a type rather than six arguments because the SEAM above owns the two +// IT IS A TYPE RATHER THAN SIX ARGUMENTS because the SEAM above owns the two // things a test cannot have — a database on the person's disk and a connection // to a provider — and this owns everything else. Splitting them there is what // lets the gate, the plan and every refusal be tested without either. +// +// OWNERS IS THE STRICT WRITE POLICY: only rows whose owner is in this list may +// be refined or superseded. The pass never reads quarantine rows, never mutates +// them, and never crosses owner boundaries with a plan that mentions another +// owner's id. A nil or empty owners list fails closed with no work done, which +// is the safe direction: a pass that cannot prove what it is authorized to touch +// must never touch everything. type tidyPass struct { brain *store.Store completer Completer model string root string mark consolidateMark + owners []string } -// run is the pass proper: read what is remembered, decide whether it is worth a -// call, make it, apply what came back, stamp the clock and say one line. +// run is the pass proper: read what is remembered within the OWNERS this pass +// is allowed to touch, decide whether it is worth a call, make it, apply what +// came back, stamp the clock and say one line. +// +// THE BATCH NEVER LEAVES THE AUTHORIZED OWNERS. The read is owner-scoped the +// way every read that can put a memory in front of a model is, and the +// quarantine — rows whose owner nobody can prove — is never in the list. A +// nil or empty owners list is a fail-closed no-op: a pass that cannot prove +// what it is allowed to touch does no work at all. func (p tidyPass) run(ctx context.Context) (standing.Tidied, error) { - batch, err := p.brain.ListMemories("", consolidateBatch) + if len(p.owners) == 0 { + return standing.Tidied{}, errors.New("session: the tidy has no authorized owners to tidy") + } + for _, owner := range p.owners { + if !store.ValidOwner(owner) || owner == store.OwnerLegacyProject { + return standing.Tidied{}, errors.New("session: the tidy has an unauthorized owner") + } + } + batch, err := p.brain.ListMemories(p.owners, consolidateBatch) if err != nil { return standing.Tidied{}, err } @@ -280,15 +330,19 @@ func (p tidyPass) run(ctx context.Context) (standing.Tidied, error) { if err != nil { return standing.Tidied{USD: usd}, err } - tidied := applyConsolidatePlan(p.brain, batch, plan) + tidied, err := applyConsolidatePlan(p.brain, batch, p.owners, plan) tidied.USD = usd + if err != nil { + noticeLiveWindow("memory tidy failed · changes remain due for review") + return tidied, err + } // THE WATERMARK IS TAKEN AFTER THE WRITES, from the store rather than from // the batch, so the pass's own updates are not what the next pass reads as // "something changed". A pass that re-triggered itself would tidy the same // fifty lines every six hours forever. _ = writeConsolidateMark(p.root, consolidateMark{ - At: time.Now(), Seq: consolidateHighWater(p.brain, batch), + At: time.Now(), Seq: consolidateHighWater(p.brain, p.owners, batch), Merged: tidied.Merged, Superseded: tidied.Superseded, USD: usd, }) @@ -312,18 +366,19 @@ func consolidateChanged(batch []store.Memory, since int64) int { return changed } -// consolidateHighWater is the newest sequence the store holds after a pass. It -// falls back to the batch's own highest when the read fails, which is the safe -// direction: a watermark that is too NEW skips a pass, where one that is too old -// pays for one that changes nothing. -func consolidateHighWater(brain *store.Store, batch []store.Memory) int64 { +// consolidateHighWater is the newest sequence the store holds after a pass, +// scoped to the pass's authorized owners. It falls back to the batch's own +// highest when the read fails, which is the safe direction: a watermark that is +// too NEW skips a pass, where one that is too old pays for one that changes +// nothing. +func consolidateHighWater(brain *store.Store, owners []string, batch []store.Memory) int64 { high := int64(0) for _, memory := range batch { if memory.UpdatedSeq > high { high = memory.UpdatedSeq } } - newest, err := brain.ListMemories("", 1) + newest, err := brain.ListMemories(owners, 1) if err == nil && len(newest) == 1 && newest[0].UpdatedSeq > high { high = newest[0].UpdatedSeq } @@ -437,21 +492,31 @@ func consolidateScopeWord(scope string) string { // ── applying the plan ─────────────────────────────────────────────────────── -// applyConsolidatePlan writes the plan through the store's existing events and -// answers what actually landed. +// applyConsolidatePlan writes the plan through the store's OWNER-GUARDED events +// and answers what actually landed. // -// EVERY REFUSAL IS SILENT AND LOCAL. An operation naming a row that is not in -// the batch, a row already touched by this plan, an empty title or a retirement -// the pass is not allowed to make changes nothing and stops nothing else in the -// plan — the model answered about fifty lines and got one of them wrong, which -// is not a reason to throw away the other seven. -func applyConsolidatePlan(brain *store.Store, batch []store.Memory, plan consolidatePlan) standing.Tidied { +// Invalid model operations are skipped locally. Store write failures are +// journaled and returned, preserving successful partial work without stamping +// the pass as complete. Mutation doors use the row's observed owner alone. +func applyConsolidatePlan(brain *store.Store, batch []store.Memory, owners []string, plan consolidatePlan) (standing.Tidied, error) { known := map[string]store.Memory{} for _, memory := range batch { known[memory.ID] = memory } + authorized := map[string]bool{} + for _, owner := range owners { + owner = strings.TrimSpace(owner) + if store.ValidOwner(owner) && owner != store.OwnerLegacyProject { + authorized[owner] = true + } + } touched := map[string]bool{} var tidied standing.Tidied + var failures error + recordFailure := func(id string, err error) { + failure := fmt.Errorf("tidy memory %s: %w", id, err) + failures = errors.Join(failures, failure, brain.JournalMemoryFailure(consolidateSource, failure)) + } for index, op := range plan.Ops { if index >= consolidateOps { break @@ -461,6 +526,13 @@ func applyConsolidatePlan(brain *store.Store, batch []store.Memory, plan consoli if !found || touched[id] { continue } + // THE OWNER BOUNDARY IS CHECKED BEFORE ANY WRITE. A row whose owner is + // not among the authorized owners — a project the pass was not built + // for, or the quarantine — is refused here even though it sat in the + // batch. The pass never mutates what it was not authorized to. + if !authorized[row.Owner] { + continue + } title, text := consolidateBody(op, row) if title == "" || text == "" { continue @@ -473,7 +545,8 @@ func applyConsolidatePlan(brain *store.Store, batch []store.Memory, plan consoli // screen about a store that did not move. continue } - if err := brain.UpdateMemoryFromSession(id, title, text, row.Tags, consolidateSource); err != nil { + if err := brain.UpdateMemoryForOwners([]string{row.Owner}, id, title, text, row.Tags, consolidateSource); err != nil { + recordFailure(id, err) continue } touched[id] = true @@ -483,14 +556,17 @@ func applyConsolidatePlan(brain *store.Store, batch []store.Memory, plan consoli continue } fresh := store.Memory{ - Type: row.Type, Scope: row.Scope, + Owner: row.Owner, Type: row.Type, Scope: row.Scope, Title: title, Text: text, Tags: row.Tags, SourceSession: consolidateSource, } // THE TYPE AND THE SCOPE ARE THE RETIRED ROW'S. A replacement is the // same memory said better; one that changed how far its truth // reaches would be a different memory wearing the old one's place. - if _, err := brain.SupersedeMemory(id, fresh); err != nil { + // The owner is carried over the same way, and the ForOwners door + // re-proves it inside the transaction. + if _, err := brain.SupersedeMemoryForOwners([]string{row.Owner}, id, fresh); err != nil { + recordFailure(id, err) continue } touched[id] = true @@ -499,7 +575,7 @@ func applyConsolidatePlan(brain *store.Store, batch []store.Memory, plan consoli continue } } - return tidied + return tidied, failures } // consolidateBody is the title and text an operation leaves behind, trimmed and diff --git a/internal/session/memory_consolidate_test.go b/internal/session/memory_consolidate_test.go index e5eafdad65..4ce273d8e1 100644 --- a/internal/session/memory_consolidate_test.go +++ b/internal/session/memory_consolidate_test.go @@ -23,13 +23,17 @@ import ( // nobody is watching, which is the failure this file exists to hold shut. // tidyScript answers the consolidation call and counts it. It knows the call by -// its system prompt, which is how the calls really differ on the wire. +// its system prompt, which is how the calls really differ on the wire, and it +// keeps the user listing it was handed so a test can assert exactly what the +// model was shown. type tidyScript struct { - mu sync.Mutex - plan string - fail error - usd float64 - calls int + mu sync.Mutex + plan string + fail error + usd float64 + calls int + listed string + beforeAnswer func() } func (s *tidyScript) CompleteWithMessages(_ context.Context, messages []ai.Message, _ ...ai.Option) (*ai.Response, error) { @@ -40,9 +44,15 @@ func (s *tidyScript) CompleteWithMessages(_ context.Context, messages []ai.Messa if !strings.Contains(system, "You tidy a person's remembered notes") { return nil, errors.New("the tidy asked something that was not the tidy") } + if s.beforeAnswer != nil { + s.beforeAnswer() + } s.mu.Lock() defer s.mu.Unlock() s.calls++ + if len(messages) > 1 { + s.listed = messageText(messages[1]) + } if s.fail != nil { return nil, s.fail } @@ -60,9 +70,22 @@ func (s *tidyScript) count() int { return s.calls } +func (s *tidyScript) listing() string { + s.mu.Lock() + defer s.mu.Unlock() + return s.listed +} + // tidyBrain is a store of the test's own with the given rows in it, oldest -// first, and the pass that reads it. +// first, and the pass that reads it. The owners default to user+machine so a +// test can observe what happens inside one partition, and may be overridden +// when it wants a specific owner policy. func tidyBrain(t *testing.T, script Completer, rows ...store.Memory) (tidyPass, *store.Store, string) { + t.Helper() + return tidyBrainWithOwners(t, script, nil, rows...) +} + +func tidyBrainWithOwners(t *testing.T, script Completer, owners []string, rows ...store.Memory) (tidyPass, *store.Store, string) { t.Helper() root := t.TempDir() brain, err := store.Open(filepath.Join(root, "brain.db")) @@ -81,12 +104,15 @@ func tidyBrain(t *testing.T, script Completer, rows ...store.Memory) (tidyPass, t.Fatalf("add memory %q: %v", row.Title, err) } } - return tidyPass{brain: brain, completer: script, model: "tidy-model", root: root}, brain, root + if len(owners) == 0 { + owners = []string{store.OwnerUser, store.OwnerMachine} + } + return tidyPass{brain: brain, completer: script, model: "tidy-model", root: root, owners: owners}, brain, root } func activeTitles(t *testing.T, brain *store.Store) []string { t.Helper() - rows, err := brain.ListMemories("", 50) + rows, err := brain.ListMemories(nil, 50) if err != nil { t.Fatalf("list memories: %v", err) } @@ -175,7 +201,7 @@ func TestTheTidyNeedsTwoThingsToHaveChanged(t *testing.T) { store.Memory{Title: "deploys on Fridays", Text: "they deploy on Fridays"}, store.Memory{Title: "prefers tabs", Text: "they prefer tabs"}, ) - rows, err := brain.ListMemories("", 50) + rows, err := brain.ListMemories(nil, 50) if err != nil { t.Fatalf("list memories: %v", err) } @@ -208,7 +234,7 @@ func TestARefineRewritesOneLineInPlace(t *testing.T) { store.Memory{Title: "tabs", Text: "they prefer tabs"}, store.Memory{Title: "tabs in go", Text: "they prefer tabs in go files"}, ) - rows, _ := brain.ListMemories("", 50) + rows, _ := brain.ListMemories(nil, 50) target := rows[0] script.plan = mustPlan(t, consolidateOp{Op: "refine", ID: target.ID, Title: "prefers tabs", Text: "they prefer tabs, in go files and everywhere else"}) @@ -243,7 +269,7 @@ func TestASupersedeRetiresTheRowAndLeavesItReadable(t *testing.T) { store.Memory{Type: store.MemoryProjectState, Title: "on v2", Text: "the api is on v2"}, store.Memory{Title: "deploys on Fridays", Text: "they deploy on Fridays"}, ) - rows, _ := brain.ListMemories("", 50) + rows, _ := brain.ListMemories(nil, 50) var target store.Memory for _, row := range rows { if row.Type == store.MemoryProjectState { @@ -291,7 +317,7 @@ func TestTheTidyNeverRetiresSomebodysOwnWords(t *testing.T) { store.Memory{Type: kind, Title: "no force pushes", Text: "never force push to main"}, store.Memory{Title: "deploys on Fridays", Text: "they deploy on Fridays"}, ) - rows, _ := brain.ListMemories("", 50) + rows, _ := brain.ListMemories(nil, 50) var mine store.Memory for _, row := range rows { if row.Type == kind { @@ -344,7 +370,7 @@ func TestABadOperationIsSkippedAndTheRestOfThePlanLands(t *testing.T) { store.Memory{Title: "one", Text: "the first thing"}, store.Memory{Title: "two", Text: "the second thing"}, ) - rows, _ := brain.ListMemories("", 50) + rows, _ := brain.ListMemories(nil, 50) script.plan = mustPlan(t, consolidateOp{Op: "refine", ID: "no-such-memory", Title: "ghost", Text: "nothing"}, consolidateOp{Op: "sideways", ID: rows[0].ID, Title: "wrong", Text: "wrong"}, @@ -377,7 +403,7 @@ func TestOnePassChangesAtMostEightLines(t *testing.T) { }) } pass, brain, _ := tidyBrain(t, script, rows...) - stored, _ := brain.ListMemories("", 50) + stored, _ := brain.ListMemories(nil, 50) ops := make([]consolidateOp, 0, len(stored)) for _, row := range stored { ops = append(ops, consolidateOp{Op: "refine", ID: row.ID, Title: row.Title + "!", Text: row.Text + "!"}) @@ -404,7 +430,7 @@ func TestAPassStampsTheClockPastItsOwnWrites(t *testing.T) { store.Memory{Title: "one", Text: "the first thing"}, store.Memory{Title: "two", Text: "the second thing"}, ) - rows, _ := brain.ListMemories("", 50) + rows, _ := brain.ListMemories(nil, 50) script.plan = mustPlan(t, consolidateOp{Op: "refine", ID: rows[0].ID, Title: "one, clearly", Text: "said once"}) tidied, err := pass.run(context.Background()) @@ -421,7 +447,7 @@ func TestAPassStampsTheClockPastItsOwnWrites(t *testing.T) { if mark.Merged != 1 || mark.USD != 0.0021 { t.Fatalf("the watermark says %+v", mark) } - after, _ := brain.ListMemories("", 1) + after, _ := brain.ListMemories(nil, 1) if mark.Seq < after[0].UpdatedSeq { t.Fatalf("the watermark is at %d, behind the pass's own write at %d", mark.Seq, after[0].UpdatedSeq) } @@ -495,6 +521,326 @@ func TestTheListingGroupsByHowFarTheTruthReaches(t *testing.T) { } } +// ── the owner boundary ────────────────────────────────────────────────────── + +// THE QUARANTINE IS NEVER IN THE MODEL'S BATCH AND NEVER MUTATED. A quarantined +// row's owner is unprovable, so the architecture says nothing injects it; an +// idle pass that read or rewrote one would be exactly the widening the +// quarantine exists to close. The listing the model sees excludes it, and even +// a plan that names its id changes nothing. +func TestTheTidyNeverReadsOrMutatesTheQuarantine(t *testing.T) { + script := &tidyScript{} + pass, brain, _ := tidyBrain(t, script, + store.Memory{Title: "user truth", Text: "true about the person"}, + store.Memory{Owner: store.OwnerMachine, Scope: store.MemoryScopeEnv, Type: store.MemoryFact, + Title: "machine truth", Text: "true about this machine"}, + store.Memory{Owner: store.OwnerLegacyProject, Scope: store.MemoryScopeProject, Type: store.MemoryFact, + Title: "orphan", Text: "a row nobody can prove the owner of"}, + ) + // Find the quarantined row's id. + all, err := brain.ListMemories(nil, 50) + if err != nil { + t.Fatal(err) + } + var orphan store.Memory + for _, row := range all { + if row.Owner == store.OwnerLegacyProject { + orphan = row + } + } + if orphan.ID == "" { + t.Fatal("the fixture did not create a quarantined row") + } + before, _, err := brain.MemoryRecord(orphan.ID) + if err != nil { + t.Fatal(err) + } + + // The model's plan names the quarantined row: a refine and a supersede. + // Neither may land. + script.plan = mustPlan(t, + consolidateOp{Op: "refine", ID: orphan.ID, Title: "rewritten", Text: "the idle pass rewrote this"}, + consolidateOp{Op: "supersede", ID: orphan.ID, Title: "replacement", Text: "the idle pass replaced this"}, + ) + tidied, err := pass.run(context.Background()) + if err != nil { + t.Fatalf("run: %v", err) + } + if tidied.Changed() != 0 { + t.Fatalf("the pass changed %d lines against quarantine, want 0", tidied.Changed()) + } + // The call WAS made — with two in-scope rows the pass had something to ask + // about — and the listing it was shown never named the quarantined row. + if script.count() != 1 { + t.Fatalf("the pass made %d calls, want 1", script.count()) + } + if strings.Contains(script.listing(), orphan.ID) || strings.Contains(script.listing(), "orphan") { + t.Fatalf("the model was shown a quarantined row:\n%s", script.listing()) + } + after, found, err := brain.MemoryRecord(orphan.ID) + if err != nil || !found { + t.Fatalf("the quarantined row vanished: %v", err) + } + if after.Title != before.Title || after.Text != before.Text || after.Status != store.MemoryActive { + t.Fatalf("the quarantined row was touched: %q / %q / %s", after.Title, after.Text, after.Status) + } +} + +// A PASS SCOPED TO ONE PROJECT NEVER TOUCHES ANOTHER PROJECT'S ROW, nor the +// person's own row, even when the model names a foreign id. The batch only ever +// held rows the pass owns, and the write door re-proves the owner in the same +// transaction. +func TestTheTidyScopedToOneProjectRefusesAnotherProjectsAndTheUsersRows(t *testing.T) { + script := &tidyScript{} + // Build the pass with ONLY project-alpha's owner authorized. + alphaOwner := store.OwnerProject("alpha-key") + pass, brain, _ := tidyBrainWithOwners(t, script, []string{alphaOwner}, + store.Memory{Owner: alphaOwner, Scope: store.MemoryScopeProject, Type: store.MemoryProjectState, + Title: "alpha on v2", Text: "the alpha api is on v2"}, + store.Memory{Owner: alphaOwner, Scope: store.MemoryScopeProject, Type: store.MemoryProjectState, + Title: "alpha deploy day", Text: "alpha deploys on Tuesdays"}, + store.Memory{Owner: store.OwnerProject("beta-key"), Scope: store.MemoryScopeProject, Type: store.MemoryProjectState, + Title: "beta on v1", Text: "the beta api is on v1"}, + store.Memory{Owner: store.OwnerUser, Scope: store.MemoryScopeUser, Type: store.MemoryFact, + Title: "user fact", Text: "true about the person"}, + ) + all, err := brain.ListMemories(nil, 50) + if err != nil { + t.Fatal(err) + } + var alpha, beta, user store.Memory + for _, row := range all { + switch row.Owner { + case alphaOwner: + if row.Title == "alpha on v2" { + alpha = row + } + case store.OwnerProject("beta-key"): + beta = row + case store.OwnerUser: + user = row + } + } + if alpha.ID == "" || beta.ID == "" || user.ID == "" { + t.Fatal("the fixture did not create all three rows") + } + + // The model names ALL THREE ids: its refine of alpha should land, its + // refine of beta and its supersede of the user row must be refused. + script.plan = mustPlan(t, + consolidateOp{Op: "refine", ID: alpha.ID, Title: "alpha on v2, clearly", Text: "the alpha api is on v2, now documented"}, + consolidateOp{Op: "refine", ID: beta.ID, Title: "beta on v1, clearly", Text: "the beta api is on v1, now documented"}, + consolidateOp{Op: "supersede", ID: user.ID, Title: "user fact superseded", Text: "no longer true"}, + ) + tidied, err := pass.run(context.Background()) + if err != nil { + t.Fatalf("run: %v", err) + } + if tidied.Merged != 1 || tidied.Superseded != 0 { + t.Fatalf("the pass came to %+v, want exactly one alpha merge", tidied) + } + // Alpha's own row was refined. + alphaAfter, _, err := brain.MemoryRecord(alpha.ID) + if err != nil || alphaAfter.Text != "the alpha api is on v2, now documented" { + t.Fatalf("alpha's row was not refined: (%v, %v)", alphaAfter, err) + } + // Beta's row is untouched. + betaAfter, _, err := brain.MemoryRecord(beta.ID) + if err != nil || betaAfter.Text != "the beta api is on v1" || betaAfter.Status != store.MemoryActive { + t.Fatalf("the pass touched beta's row: %q / %s", betaAfter.Text, betaAfter.Status) + } + // The person's row is untouched. + userAfter, _, err := brain.MemoryRecord(user.ID) + if err != nil || userAfter.Text != "true about the person" || userAfter.Status != store.MemoryActive { + t.Fatalf("the pass touched the user's row: %q / %s", userAfter.Text, userAfter.Status) + } + // And the model was never even SHOWN the foreign rows. + if strings.Contains(script.listing(), beta.ID) || strings.Contains(script.listing(), user.ID) { + t.Fatalf("the model was shown rows the pass does not own:\n%s", script.listing()) + } +} + +// AN OWNER REMOVED OR CHANGED BETWEEN THE READ AND THE WRITE IS REFUSED. The +// batch is read under one owner list, but the write door re-proves the row's +// CURRENT owner in the same transaction — a row that stopped being the pass's +// between listing and apply cannot be rewritten by it. +func TestTheTidyRefusesARowWhoseOwnerMovedAfterTheRead(t *testing.T) { + _, brain, _ := tidyBrain(t, &tidyScript{}, + store.Memory{Title: "one", Text: "the first thing"}, + store.Memory{Title: "two", Text: "the second thing"}, + ) + rows, err := brain.ListMemories(nil, 50) + if err != nil { + t.Fatal(err) + } + target := rows[0] + + // The batch the pass read holds a row owned by the user, but the WRITE is + // scoped to a narrower owner list that no longer includes the user — the + // shape of an owner that moved between listing and apply. The pass's own + // boundary check must refuse it, and the store's guarded door must refuse + // it too. + plan := consolidatePlan{Ops: []consolidateOp{{Op: "refine", ID: target.ID, Title: "moved", Text: "the row moved away"}}} + tidied, err := applyConsolidatePlan(brain, rows, []string{store.OwnerMachine}, plan) + if err != nil { + t.Fatalf("refusing an unauthorized plan: %v", err) + } + if tidied.Changed() != 0 { + t.Fatalf("a plan applied under the wrong owner changed %d lines", tidied.Changed()) + } + check, _, err := brain.MemoryRecord(target.ID) + if err != nil || check.Text != target.Text { + t.Fatalf("the row was changed despite the owner mismatch: %q", check.Text) + } +} + +// A MISSING OWNER LIST IS A FAIL-CLOSED NO-OP. The pass reads nothing, asks +// nothing, writes nothing — a consolidate that cannot prove what it may touch +// must not touch everything. +func TestTheTidyFailsClosedWithNoAuthorizedOwners(t *testing.T) { + script := &tidyScript{} + pass, _, _ := tidyBrain(t, script, + store.Memory{Title: "one", Text: "the first thing"}, + store.Memory{Title: "two", Text: "the second thing"}, + ) + pass.owners = nil + tidied, err := pass.run(context.Background()) + if err == nil { + t.Fatal("a pass with no authorized owners answered no error") + } + if tidied.Changed() != 0 { + t.Fatalf("a pass with no owners changed %d lines", tidied.Changed()) + } + if script.count() != 0 { + t.Fatalf("a pass with no owners made %d calls", script.count()) + } +} + +// A LEGITIMATE SAME-OWNER REFINEMENT STILL WORKS through the guarded door — +// the owner boundary is a filter on who may be touched, never a ban on tidying +// the pass's own rows. +func TestTheTidyStillRefinesItsOwnOwnersRows(t *testing.T) { + script := &tidyScript{} + pass, brain, _ := tidyBrain(t, script, + store.Memory{Title: "one", Text: "the first thing"}, + store.Memory{Title: "two", Text: "the second thing"}, + ) + rows, _ := brain.ListMemories(nil, 50) + script.plan = mustPlan(t, consolidateOp{Op: "refine", ID: rows[0].ID, Title: "one, clearly", Text: "the first thing, said once"}) + tidied, err := pass.run(context.Background()) + if err != nil { + t.Fatalf("run: %v", err) + } + if tidied.Merged != 1 { + t.Fatalf("a same-owner refine came to %+v, want one merge", tidied) + } +} + +func TestTheTidyRejectsLegacyProjectConfiguration(t *testing.T) { + for _, key := range []string{"legacy", " legacy "} { + t.Run(key, func(t *testing.T) { + script := &tidyScript{} + pass, brain, _ := tidyBrain(t, script, + store.Memory{Title: "one", Text: "first user fact"}, + store.Memory{Title: "two", Text: "second user fact"}, + store.Memory{Owner: store.OwnerLegacyProject, Title: "orphan", Text: "quarantined fact"}, + ) + rows := mustList(t, brain) + var orphan store.Memory + for _, row := range rows { + if row.Owner == store.OwnerLegacyProject { + orphan = row + } + } + if orphan.ID == "" { + t.Fatal("missing quarantine fixture") + } + pass.owners = consolidateOwners(Config{MemoryProjectKey: key}) + script.plan = mustPlan(t, consolidateOp{Op: "refine", ID: orphan.ID, Text: "rewritten"}) + if _, err := pass.run(context.Background()); err != nil { + t.Fatal(err) + } + if strings.Contains(script.listing(), orphan.ID) { + t.Fatal("legacy configuration exposed quarantine to model") + } + after, found, err := brain.MemoryRecord(orphan.ID) + if err != nil || !found || after.Text != orphan.Text { + t.Fatalf("quarantine changed: %+v, %v", after, err) + } + }) + } +} + +func TestTheTidyRefusesExplicitQuarantineAuthorization(t *testing.T) { + script := &tidyScript{} + pass, _, root := tidyBrain(t, script, + store.Memory{Owner: store.OwnerLegacyProject, Title: "one", Text: "first quarantined fact"}, + store.Memory{Owner: store.OwnerLegacyProject, Title: "two", Text: "second quarantined fact"}, + ) + pass.owners = []string{store.OwnerLegacyProject} + if _, err := pass.run(context.Background()); err == nil { + t.Fatal("quarantine authorization was accepted") + } + if script.count() != 0 { + t.Fatal("quarantine was sent to model") + } + if _, found := readConsolidateMark(root); found { + t.Fatal("refused pass stamped success") + } +} + +func TestFailedTidyWritesAreJournaledWithoutSuccessStamp(t *testing.T) { + for _, op := range []string{consolidateRefine, consolidateSupersede} { + t.Run(op, func(t *testing.T) { + script := &tidyScript{usd: 0.0021} + pass, brain, root := tidyBrain(t, script, + store.Memory{Title: "one", Text: "first fact"}, + store.Memory{Title: "two", Text: "second fact"}, + ) + rows := mustList(t, brain) + target := rows[0] + prior := consolidateMark{At: time.Now().Add(-2 * consolidateEvery)} + if err := writeConsolidateMark(root, prior); err != nil { + t.Fatal(err) + } + pass.mark = prior + script.plan = mustPlan(t, + consolidateOp{Op: op, ID: target.ID, Text: "failed replacement"}, + consolidateOp{Op: "refine", ID: rows[1].ID, Text: "successful refinement"}, + ) + script.beforeAnswer = func() { + if err := brain.ForgetMemory(target.ID); err != nil { + t.Fatal(err) + } + } + tidied, err := pass.run(context.Background()) + if err == nil || !strings.Contains(err.Error(), target.ID) { + t.Fatalf("failed write not surfaced: %v", err) + } + if tidied.Merged != 1 || tidied.Superseded != 0 || tidied.USD != script.usd { + t.Fatalf("partial result lost: %+v", tidied) + } + mark, found := readConsolidateMark(root) + if !found || !mark.At.Equal(prior.At) || mark.Seq != prior.Seq { + t.Fatalf("failed pass stamped success: %+v", mark) + } + events, err := brain.Events(0, 100) + if err != nil { + t.Fatal(err) + } + failures := 0 + for _, event := range events { + if event.Kind == store.EventMemoryWriteFailed { + failures++ + } + } + if failures != 1 { + t.Fatalf("journaled failures = %d, want 1", failures) + } + }) + } +} + // ── helpers ───────────────────────────────────────────────────────────────── func mustPlan(t *testing.T, ops ...consolidateOp) string { @@ -508,7 +854,7 @@ func mustPlan(t *testing.T, ops ...consolidateOp) string { func mustList(t *testing.T, brain *store.Store) []store.Memory { t.Helper() - rows, err := brain.ListMemories("", 50) + rows, err := brain.ListMemories(nil, 50) if err != nil { t.Fatalf("list memories: %v", err) } diff --git a/internal/session/memory_owner_test.go b/internal/session/memory_owner_test.go new file mode 100644 index 0000000000..904820c1a5 --- /dev/null +++ b/internal/session/memory_owner_test.go @@ -0,0 +1,663 @@ +package session + +// The memory contracts this wave added, proved the way this package proves +// everything: what a model was shown, what the store holds, and what the +// person could see. + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/store" +) + +// ── owner isolation, through the session's own doors ──────────────────────── + +// A memory written in one project never surfaces in another. The router's +// shortlist, the dedup search and a person's query are all scoped to the +// owners the session can see, and the session's owners come from the project +// key the door handed it. +func TestAMemoryInOneProjectNeverSurfacesInAnother(t *testing.T) { + brain, err := store.Open(filepath.Join(t.TempDir(), "shared.db")) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = brain.Close() }) + + alpha, _ := newTestAgent(t, &reflexScript{}, func(config *Config) { + config.Memory = brain + config.MemoryProjectKey = "alpha-key" + }) + beta, _ := newTestAgent(t, &reflexScript{}, func(config *Config) { + config.Memory = brain + config.MemoryProjectKey = "beta-key" + }) + + if _, err := alpha.RememberScoped("deploys to the amber cluster every Tuesday", store.MemoryScopeProject); err != nil { + t.Fatalf("alpha's project write: %v", err) + } + + // BETA'S ROUTER IS NEVER SHOWN IT. The shortlist beta's router reads is + // beta's own view; the store never offered alpha's row. + block := beta.memoryBlock(context.Background(), "where do we deploy the amber cluster") + if strings.Contains(block, "amber cluster") { + t.Fatalf("beta's block carried alpha's memory:\n%s", block) + } + // BETA'S SEARCH FINDS NOTHING OF ALPHA'S — a person asking in beta does + // not get alpha's project memory back. + found, err := beta.Memories("amber cluster") + if err != nil || len(found) != 0 { + t.Fatalf("beta's query = (%v, %v), want nothing of alpha's", found, err) + } + // AND ALPHA STILL SEES ITS OWN. + alphaFound, err := alpha.Memories("amber cluster") + if err != nil || len(alphaFound) != 1 { + t.Fatalf("alpha's query = (%v, %v), want its own row", alphaFound, err) + } + // The row itself is owned by alpha's project, not by the person at large. + rows, err := brain.SearchMemories([]string{store.OwnerProject("alpha-key")}, "amber", 5) + if err != nil || len(rows) != 1 { + t.Fatalf("alpha's owner holds %v (%v)", memoryIDs(rows), err) + } +} + +// The same words written from two projects are TWO ROWS: the dedup door never +// lets one project's write swallow another's, because two projects may hold +// genuinely different truths about the same words. +func TestTheSameWordsInTwoProjectsAreTwoMemories(t *testing.T) { + brain, err := store.Open(filepath.Join(t.TempDir(), "twotruths.db")) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = brain.Close() }) + + alpha, _ := newTestAgent(t, &reflexScript{}, func(config *Config) { + config.Memory = brain + config.MemoryProjectKey = "alpha-key" + }) + beta, _ := newTestAgent(t, &reflexScript{}, func(config *Config) { + config.Memory = brain + config.MemoryProjectKey = "beta-key" + }) + + if _, err := alpha.RememberScoped("the API key rotates every ninety days", store.MemoryScopeProject); err != nil { + t.Fatal(err) + } + if _, err := beta.RememberScoped("the API key rotates every ninety days", store.MemoryScopeProject); err != nil { + t.Fatal(err) + } + alphaRows, err := brain.SearchMemories([]string{store.OwnerProject("alpha-key")}, "rotates", 5) + if err != nil || len(alphaRows) != 1 { + t.Fatalf("alpha holds %v (%v)", memoryIDs(alphaRows), err) + } + betaRows, err := brain.SearchMemories([]string{store.OwnerProject("beta-key")}, "rotates", 5) + if err != nil || len(betaRows) != 1 { + t.Fatalf("beta holds %v (%v)", memoryIDs(betaRows), err) + } + // A write scoped to the user, from either project, is the person's own and + // is deduplicated against itself only. +} + +// A task worker inherits its parent's owners, and a worker cannot WRITE +// memories at all — the family would otherwise be eight writers on one brain, +// all blind to each other. +func TestAWorkerInheritsTheViewAndHoldsNoWriteVerb(t *testing.T) { + brain, err := store.Open(filepath.Join(t.TempDir(), "worker.db")) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = brain.Close() }) + parent, _ := newTestAgent(t, &reflexScript{}, func(config *Config) { + config.Memory = brain + config.MemoryProjectKey = "alpha-key" + }) + // THE WORKER'S SHAPE IS THE ONE THE TASK TREE BUILDS: no memory brain of + // its own ([Agent.remembers] is false on a node's workers), the parent's + // routed block handed to it. The belt check proves the ABSENCE of the + // write verb, which is what keeps a family of workers from being eight + // writers on one brain. + worker, _ := newTestAgent(t, &reflexScript{}, func(config *Config) { + config.Memory = brain + config.MemoryProjectKey = "alpha-key" + config.PromptProfile = "lean" + }) + _ = parent + for _, tool := range worker.belt() { + if tool.Name == "remember" { + t.Fatal("a worker carries the remember verb; a family of workers would be eight writers") + } + } + // AND THE VIEW A WORKER WOULD READ THROUGH IS ITS PARENT'S: the config is + // copied whole, so the owners a node's reading is scoped to are the + // parent's. + if owners := parent.memoryOwners(); len(owners) != 3 || owners[2] != store.OwnerProject("alpha-key") { + t.Fatalf("the parent's view = %v, want the project's owners", owners) + } + if owners := worker.memoryOwners(); len(owners) != 3 || owners[2] != store.OwnerProject("alpha-key") { + t.Fatalf("the worker's copied view = %v, want the parent's owners", owners) + } +} + +// ── one write door ────────────────────────────────────────────────────────── + +// Every mouth that says "keep this" walks one door, so a preference stated in +// three ways through three doors is one row, not three. +func TestEveryRememberDoorSettlesIntoOneRow(t *testing.T) { + script := &reflexScript{ + decide: `{"op":"add"}`, + // Door two rides the ROUTER, which must name the command for the door + // to be walked at all. + route: `{"inject":[],"cmd":{"name":"remember","arg":"always deploys on Fridays"}}`, + } + agent, brain := brainAgent(t, script, nil) + + // Door one: /remember. + if _, err := agent.Remember("always deploys on Fridays"); err != nil { + t.Fatalf("/remember: %v", err) + } + // Door two: the same words through the routed command — a different mouth, + // a different turn, the same row. The router was scripted to name it. + agent.routedMemory(context.Background(), "remember that I always deploy on Fridays", + nil, true) + // Door three: the same words typed with different case, which is the same + // words to the door. + if _, err := agent.Remember("ALWAYS deploys on Fridays"); err != nil { + t.Fatalf("/remember retyped: %v", err) + } + + kept, err := brain.ListMemories([]string{store.OwnerUser}, 10) + if err != nil { + t.Fatal(err) + } + if len(kept) != 1 { + t.Fatalf("three doors kept %v, want one row", titles(kept)) + } + // And the skips are journaled, so the record says what happened. + events, err := brain.Events(0, 100) + if err != nil { + t.Fatal(err) + } + skips := 0 + for _, event := range events { + if event.Kind == store.EventMemorySkipped { + skips++ + } + } + if skips != 2 { + t.Fatalf("skips journaled = %d, want 2", skips) + } +} + +// An explicit save works with NO reflex model at all: the words a person typed +// are kept by the store's own door, and a provider outage cannot lose them. +func TestAnExplicitSaveNeedsNoModel(t *testing.T) { + // THE DECIDER CANNOT ANSWER — an outage, a nonsense reply, the shape "no + // model" takes in a live session. The save still happens: the store's own + // dedup door needs no model, and the words a person typed are never + // refused because somebody else's service was down. + script := &reflexScript{decErr: errorsNew("the decider answered nonsense")} + agent, brain := brainAgent(t, script, nil) + // A NEARBY ROW is what puts the decider in the path at all: without one, + // the save needs no model and the test would prove nothing. + remember(t, brain, "deploy days", "deploys happen on some day of the week") + title, err := agent.Remember("always deploys on Fridays") + if err != nil { + t.Fatalf("/remember with a dead decider: %v", err) + } + if title == "" { + t.Fatal("the save answered no title") + } + kept, err := brain.ListMemories([]string{store.OwnerUser}, 10) + if err != nil || len(kept) != 2 { + t.Fatalf("the save without a decider = (%v, %v), want both rows", titles(kept), err) + } + // AND THE FAILED SETTLE IS JOURNALED — the save happened, the settle did + // not, and the record says so. + events, err := brain.Events(0, 100) + if err != nil { + t.Fatal(err) + } + failed := 0 + for _, event := range events { + if event.Kind == store.EventMemoryWriteFailed { + failed++ + } + } + if failed != 1 { + t.Fatalf("settle failures journaled = %d, want 1", failed) + } +} + +// ── visible failures ──────────────────────────────────────────────────────── + +// A failed extraction is journaled and said, once per window — never silent, +// never a flood. +func TestAFailedExtractionIsJournaledAndSaidOnce(t *testing.T) { + script := &reflexScript{extractErr: errorsNew("provider is down")} + agent, brain := brainAgent(t, script, nil) + script.extract = `{"mem":0}` + + // Drive the post-turn pass by hand: the failure path is the pass's, not the + // turn's. + agent.memory.setInjected(nil) + agent.learnFromTurn("remember that I prefer tabs", "I will keep that in mind.") + // The pass runs on its own goroutine; wait for the journal to carry it. + deadline := time.Now().Add(5 * time.Second) + for { + events, err := brain.Events(0, 100) + if err != nil { + t.Fatal(err) + } + found := false + for _, event := range events { + if event.Kind == store.EventMemoryWriteFailed { + found = true + } + } + if found { + break + } + if time.Now().After(deadline) { + t.Fatal("the failed extraction was never journaled") + } + time.Sleep(20 * time.Millisecond) + } + // AND THE PERSON IS TOLD — the notice is held for the next stream, which is + // where the memory lines ride. + held := agent.memory.takeNotices() + if len(held) == 0 || !strings.Contains(held[0], "couldn't settle a memory") { + t.Fatalf("the held notices = %v, want one line naming the failure", held) + } + // AND ONCE PER WINDOW: more failures inside the window say nothing more. + agent.learnFromTurn("and remember the other thing", "Noted.") + held = agent.memory.takeNotices() + if len(held) != 0 { + t.Fatalf("a second failure in the window said %v, want silence", held) + } +} + +// When the STORE itself is what failed, the reason goes to a file beside the +// state root instead of nowhere. +func TestAFailureWithADeadStoreReachesTheFallbackFile(t *testing.T) { + brain, err := store.Open(filepath.Join(t.TempDir(), "dying.db")) + if err != nil { + t.Fatal(err) + } + agent, _ := newTestAgent(t, &reflexScript{}, func(config *Config) { + config.Memory = brain + }) + if err := brain.Close(); err != nil { + t.Fatal(err) + } + // Set the window back so the notice path is exercised too. + agent.memory.failureSaid = time.Time{} + agent.journalMemoryFailure("settle", errorsNew("disk went away")) + + path := memoryFailureLogPath() + body, err := os.ReadFile(path) + if err != nil { + t.Fatalf("the fallback file was not written: %v", err) + } + if !strings.Contains(string(body), "via=settle") || !strings.Contains(string(body), "disk went away") { + t.Fatalf("the fallback line = %q, want the via and the reason", body) + } +} + +// ── the legacy import ─────────────────────────────────────────────────────── + +// An import that cannot read the whole file leaves the file exactly where it +// is, journals why, and retries on the next call. +func TestAFailedImportLeavesTheFileAndRetries(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "memory.md") + // A line longer than any buffer the scanner will grow into: the scan fails + // at it, which is the failure the old importer renamed over. It stands + // FIRST, so nothing after it was ever read. + var long strings.Builder + for index := 0; index < 70_000; index++ { + long.WriteString("x") + } + body := long.String() + "\na good line worth keeping\nanother good line\n" + if err := os.WriteFile(path, []byte(body), 0o600); err != nil { + t.Fatal(err) + } + + script := &reflexScript{} + agent, brain := brainAgent(t, script, func(config *Config) { + config.MemoryImport = path + }) + + agent.importMemoryFile() + kept, err := brain.ListMemories([]string{store.OwnerUser}, 10) + if err != nil { + t.Fatal(err) + } + if len(kept) != 0 { + t.Fatalf("a scan failure imported %v; the file is not proven readable", titles(kept)) + } + if _, err := os.Stat(path); err != nil { + t.Fatalf("the file was moved out of the way after a scan failure: %v", err) + } + if agent.memory.imported { + t.Fatal("a failed import left its once-marker set; the next turn would not retry") + } + + // THE FILE IS FIXED — smaller lines now — and the retry imports everything. + if err := os.WriteFile(path, []byte("a good line worth keeping\nanother good line\n"), 0o600); err != nil { + t.Fatal(err) + } + agent.memory.imported = false + agent.importMemoryFile() + kept, err = brain.ListMemories([]string{store.OwnerUser}, 10) + if err != nil || len(kept) != 2 { + t.Fatalf("the retry imported (%v, %v), want both lines", titles(kept), err) + } + if _, err := os.Stat(path + ".imported"); err != nil { + t.Fatalf("the imported file was not renamed: %v", err) + } + + // AND THE RETRY IS IDEMPOTENT: running the import again — the state a + // rename that failed would leave — adds nothing. + agent.memory.imported = false + agent.importMemoryFile() + kept, err = brain.ListMemories([]string{store.OwnerUser}, 10) + if err != nil || len(kept) != 2 { + t.Fatalf("a repeated import holds (%v, %v), want the same two rows", titles(kept), err) + } +} + +// A PARTIAL import — one row refused — leaves the file and says so; the rows +// that landed stay landed, and the retry skips them as duplicates. +func TestAnImportSurvivesADeadDeciderWithoutDuplicates(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "memory.md") + if err := os.WriteFile(path, []byte("first good line\nsecond good line\n"), 0o600); err != nil { + t.Fatal(err) + } + // A row whose settle the decider refuses — the same shape a provider + // outage makes of a real import. THE SAVE IS LOSSLESS ANY MORE: the + // decider failing does not refuse the row, because the words are the + // person's and the store's own door does not need a model. + script := &reflexScript{decErr: errorsNew("the decider answered nonsense")} + agent, brain := brainAgent(t, script, func(config *Config) { + config.MemoryImport = path + }) + seed := remember(t, brain, "earlier line", "first good line") + _ = seed + + agent.importMemoryFile() + // THE FILE WAS IMPORTED ANYWAY — every row landed through the store's own + // door — and the failed settles are in the journal, not vanished. + if _, err := os.Stat(path + ".imported"); err != nil { + t.Fatalf("the import did not finish past a dead decider: %v", err) + } + kept, err := brain.ListMemories([]string{store.OwnerUser}, 10) + if err != nil || len(kept) != 2 { + // The seeded row IS "first good line", so the first line's save skips + // as its duplicate and only the second lands fresh. + t.Fatalf("the import past a dead decider holds (%v, %v), want seed + the second line", titles(kept), err) + } + // The settle failures are journaled, one per row the decider was asked. + events, err := brain.Events(0, 100) + if err != nil { + t.Fatal(err) + } + failed := 0 + for _, event := range events { + if event.Kind == store.EventMemoryWriteFailed { + var payload map[string]string + _ = json.Unmarshal(event.Payload, &payload) + if payload["via"] == "explicit-settle" { + failed++ + } + } + } + if failed != 2 { + t.Fatalf("settle failures journaled = %d, want 2 — one per row the decider refused", failed) + } + + // AND A RE-IMPORT ADDS NOTHING: the door's dedup is what makes the retry + // idempotent, whatever killed the first attempt. + agent.memory.imported = false + agent.importMemoryFile() + kept, err = brain.ListMemories([]string{store.OwnerUser}, 10) + if err != nil || len(kept) != 2 { + t.Fatalf("a repeated import holds (%v, %v), want the same two rows", titles(kept), err) + } +} + +// ── memory off is not search off ──────────────────────────────────────────── + +// A session with memory OFF and an index still carries the search verb and no +// memory verb. The memory row decides what is REMEMBERED; the conversation +// index decides what is SEARCHABLE. +func TestMemoryOffWithAnIndexKeepsSearchAndLosesRemember(t *testing.T) { + index, err := store.Open(filepath.Join(t.TempDir(), "index.db")) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = index.Close() }) + agent, _ := newTestAgent(t, &reflexScript{}, func(config *Config) { + config.MemoryIndex = index + }) + var search, rememberTool bool + for _, tool := range agent.belt() { + switch tool.Name { + case "search_conversations": + search = true + case "remember": + rememberTool = true + } + } + if !search { + t.Fatal("a session with an index and memory off has no search verb") + } + if rememberTool { + t.Fatal("a session with memory off carries the remember verb") + } + // AND THE CONVERSATION ITSELF IS INDEXED, so the next search can find it: + // the journal posts into the index store, not into a nil memory. + if agent.chatlog == nil { + t.Fatal("memory off closed the chat journal; the conversation would never be searchable") + } +} + +func memoryIDs(memories []store.Memory) []string { + ids := make([]string, len(memories)) + for index, memory := range memories { + ids[index] = memory.ID + } + return ids +} + +// errorsNew keeps the test file's error spelling in one place. +func errorsNew(text string) error { return &failureError{text} } + +type failureError struct{ text string } + +func (e *failureError) Error() string { return e.text } + +// ── regression: foreign-ID write isolation through the session ──────────────── + +// A decider that names another project's memory id — from a stale neighbor +// list, a replayed conversation, or a model invention — cannot update that +// memory through the session's write path. The store-level owner guard +// refuses even if the session's own visibility check were bypassed. +func TestAForeignIDCannotUpdateAnotherProjectsMemory(t *testing.T) { + brain, err := store.Open(filepath.Join(t.TempDir(), "foreign-update.db")) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = brain.Close() }) + + alpha, _ := newTestAgent(t, &reflexScript{}, func(config *Config) { + config.Memory = brain + config.MemoryProjectKey = "alpha-key" + }) + + // Write a memory from alpha's project. + title, err := alpha.RememberScoped("deploys to the amber cluster on Tuesdays", store.MemoryScopeProject) + if err != nil { + t.Fatal(err) + } + if title == "" { + t.Fatal("alpha's project write produced no title") + } + + // Find the row directly. + alphaRows, err := brain.SearchMemories([]string{store.OwnerProject("alpha-key")}, "amber", 5) + if err != nil || len(alphaRows) != 1 { + t.Fatalf("alpha's rows = (%v, %v)", memoryIDs(alphaRows), err) + } + foreignID := alphaRows[0].ID + + // A beta agent with a different project key tries to update alpha's row + // through the scoped store door. + beta, _ := newTestAgent(t, &reflexScript{}, func(config *Config) { + config.Memory = brain + config.MemoryProjectKey = "beta-key" + }) + betaOwners := beta.memoryOwners() + err = brain.UpdateMemoryForOwners(betaOwners, foreignID, + "Deploys Wednesdays", "Deploys to the ember cluster on Wednesdays.", nil, "") + if err == nil { + t.Fatal("UpdateMemoryForOwners accepted a foreign project id") + } + + // Alpha's row is unchanged. + alphaRows2, err := brain.SearchMemories([]string{store.OwnerProject("alpha-key")}, "amber", 5) + if err != nil || len(alphaRows2) != 1 { + t.Fatalf("alpha's rows after foreign update = (%v, %v)", memoryIDs(alphaRows2), err) + } + if alphaRows2[0].Text != alphaRows[0].Text { + t.Fatalf("alpha's row was modified: %q", alphaRows2[0].Text) + } +} + +// A decider that names another project's memory id cannot supersede that +// memory through the session's write path. The store-level owner guard +// refuses, and the old row stays active. +func TestAForeignIDCannotSupersedeAnotherProjectsMemory(t *testing.T) { + brain, err := store.Open(filepath.Join(t.TempDir(), "foreign-supersede.db")) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = brain.Close() }) + + alpha, _ := newTestAgent(t, &reflexScript{}, func(config *Config) { + config.Memory = brain + config.MemoryProjectKey = "alpha-key" + }) + + title, err := alpha.RememberScoped("deploys to the amber cluster on Tuesdays", store.MemoryScopeProject) + if err != nil { + t.Fatal(err) + } + if title == "" { + t.Fatal("alpha's project write produced no title") + } + + alphaRows, err := brain.SearchMemories([]string{store.OwnerProject("alpha-key")}, "amber", 5) + if err != nil || len(alphaRows) != 1 { + t.Fatalf("alpha's rows = (%v, %v)", memoryIDs(alphaRows), err) + } + foreignID := alphaRows[0].ID + + // A beta agent tries to supersede alpha's row through the scoped store door. + beta, _ := newTestAgent(t, &reflexScript{}, func(config *Config) { + config.Memory = brain + config.MemoryProjectKey = "beta-key" + }) + betaOwners := beta.memoryOwners() + _, err = brain.SupersedeMemoryForOwners(betaOwners, foreignID, store.Memory{ + Owner: store.OwnerProject("beta-key"), Type: store.MemoryFact, + Title: "Deploys Wednesdays", Text: "Deploys to the ember cluster on Wednesdays.", + }) + if err == nil { + t.Fatal("SupersedeMemoryForOwners accepted a foreign project id") + } + + // Alpha's row is still active. + alphaRows2, err := brain.SearchMemories([]string{store.OwnerProject("alpha-key")}, "amber", 5) + if err != nil || len(alphaRows2) != 1 { + t.Fatalf("alpha's rows after foreign supersede = (%v, %v)", memoryIDs(alphaRows2), err) + } + if alphaRows2[0].Status != store.MemoryActive { + t.Fatalf("alpha's row is %q, want active", alphaRows2[0].Status) + } +} + +// The post-turn extraction that infers a project-scoped memory after a /forget +// removed the project copy must not auto-widen that memory to user-global. The +// extraction ownership is pinned to the session's authorized audience — the +// project key that the door proved. A memory scoped to project by the model +// stays in the project, even when no project copy exists to dedup against. +func TestPostTurnExtractionDoesNotWidenProjectMemoryToUserGlobal(t *testing.T) { + brain, err := store.Open(filepath.Join(t.TempDir(), "no-widen.db")) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = brain.Close() }) + + alpha, _ := newTestAgent(t, &reflexScript{}, func(config *Config) { + config.Memory = brain + config.MemoryProjectKey = "alpha-key" + }) + + // Write a project-scoped memory. + title, err := alpha.RememberScoped("the deploy window opens on Tuesday morning", store.MemoryScopeProject) + if err != nil { + t.Fatal(err) + } + if title == "" { + t.Fatal("project write produced no title") + } + + // Forget it from the project's view. + forgot, err := alpha.Forget("deploy window") + if err != nil { + t.Fatal(err) + } + if forgot == "" { + t.Fatal("forget found nothing") + } + + // Re-member the same content. The session's ownerForScope pins the + // project scope to the authorized project, never widens to user. + title2, err := alpha.RememberScoped("the deploy window opens on Tuesday morning", store.MemoryScopeProject) + if err != nil { + t.Fatal(err) + } + if title2 == "" { + t.Fatal("second project write produced no title") + } + + // The new row is owned by alpha's project, NOT by user. + allRows, err := brain.ListMemories(nil, 10) + if err != nil { + t.Fatal(err) + } + for _, row := range allRows { + if row.Status != store.MemoryActive { + continue + } + if strings.Contains(row.Text, "deploy window") && row.Owner == store.OwnerUser { + t.Fatalf("a project-scoped memory landed as user-global: owner=%q text=%q", row.Owner, row.Text) + } + } + + // And user-scoped searches in another project do not find it. + beta, _ := newTestAgent(t, &reflexScript{}, func(config *Config) { + config.Memory = brain + config.MemoryProjectKey = "beta-key" + }) + block := beta.memoryBlock(context.Background(), "deploy window Tuesday") + if strings.Contains(block, "deploy window") { + t.Fatalf("beta's block carried alpha's project memory after /forget re-write:\n%s", block) + } +} diff --git a/internal/session/memory_test.go b/internal/session/memory_test.go index 2959922d0a..4bf94c719e 100644 --- a/internal/session/memory_test.go +++ b/internal/session/memory_test.go @@ -317,7 +317,7 @@ func TestTheRoutersRememberCommandWritesToTheStore(t *testing.T) { agent.routedMemory(context.Background(), "remember that I always deploy on Fridays", func(line string) { seen = append(seen, line) }, true) - kept, err := brain.ListMemories("", 10) + kept, err := brain.ListMemories(nil, 10) if err != nil { t.Fatalf("list: %v", err) } @@ -346,7 +346,7 @@ func TestTheRoutersForgetCommandDropsTheMatchAndSaysWhich(t *testing.T) { agent.routedMemory(context.Background(), "forget when standup is", nil, true) - kept, err := brain.ListMemories("", 10) + kept, err := brain.ListMemories(nil, 10) if err != nil { t.Fatalf("list: %v", err) } @@ -369,7 +369,7 @@ func TestForgettingWhatIsNotThereSaysSo(t *testing.T) { if len(seen) != 1 || !strings.Contains(seen[0], "nothing matched") { t.Fatalf("the answer was %v, want one line saying nothing matched", seen) } - kept, _ := brain.ListMemories("", 10) + kept, _ := brain.ListMemories(nil, 10) if len(kept) != 1 { t.Fatalf("a no-match forget changed the store: %v", titles(kept)) } @@ -428,7 +428,7 @@ func TestAnExchangeWorthKeepingLandsWithNoDecisionToMake(t *testing.T) { t.Fatalf("close: %v", err) } - kept, err := brain.ListMemories("", 10) + kept, err := brain.ListMemories(nil, 10) if err != nil { t.Fatalf("list: %v", err) } @@ -497,7 +497,7 @@ func TestANeighbourTurnsTheWriteIntoADecision(t *testing.T) { t.Fatalf("close: %v", err) } - kept, err := brain.ListMemories("", 10) + kept, err := brain.ListMemories(nil, 10) if err != nil { t.Fatalf("list: %v", err) } @@ -514,7 +514,7 @@ func TestAnExchangeWithNothingInItWritesNothing(t *testing.T) { collect(t, mustSubmit(t, agent, "what is the capital of France")) _ = agent.Close() - kept, _ := brain.ListMemories("", 10) + kept, _ := brain.ListMemories(nil, 10) if len(kept) != 0 { t.Fatalf("an exchange worth nothing wrote %v", titles(kept)) } @@ -529,7 +529,7 @@ func TestExtractedMemoryCarriesTheSessionID(t *testing.T) { sessionID := agent.memorySourceSession() collect(t, mustSubmit(t, agent, "I prefer tabs in Go")) _ = agent.Close() - kept, err := brain.ListMemories("", 10) + kept, err := brain.ListMemories(nil, 10) if err != nil || len(kept) != 1 || kept[0].SourceSession != sessionID || kept[0].SourceSeq == 0 { t.Fatalf("extracted memory = (%+v, %v), want source session %q", kept, err, sessionID) } @@ -547,7 +547,7 @@ func TestAFailedExtractionBreaksNothingAndWritesNothing(t *testing.T) { } _ = agent.Close() - kept, _ := brain.ListMemories("", 10) + kept, _ := brain.ListMemories(nil, 10) if len(kept) != 0 { t.Fatalf("a failed extraction wrote %v", titles(kept)) } @@ -674,7 +674,7 @@ func TestTheOldMemoryFileIsImportedOnceAndRenamed(t *testing.T) { agent.importMemoryFile() - kept, err := brain.ListMemories("", 10) + kept, err := brain.ListMemories(nil, 10) if err != nil { t.Fatalf("list: %v", err) } @@ -694,7 +694,7 @@ func TestTheOldMemoryFileIsImportedOnceAndRenamed(t *testing.T) { config.MemoryImport = path }) second.importMemoryFile() - again, _ := brain.ListMemories("", 10) + again, _ := brain.ListMemories(nil, 10) if len(again) != 2 { t.Fatalf("a second run imported again: %v", titles(again)) } diff --git a/internal/session/session.go b/internal/session/session.go index 72bb398f31..35ebcf26a8 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -1329,6 +1329,31 @@ type Config struct { // and every machine that has already been through it. MemoryImport string + // MemoryProjectKey is THIS SESSION'S PROVABLE PROJECT IDENTITY — the key + // internal/gitidentity.ProjectKey mints from the workspace's origin remote + // (or its canonical path, when there is no remote) — and it is what turns + // the scope word `project` into the owner a memory really belongs to. + // Empty means the door could not prove a project, and a project write from + // this session lands in quarantine rather than somewhere guessed + // (memory.go's ownerForScope). + // + // IT IS A CONFIG FIELD AND NOT A STORE FIELD for the reason every other + // where-does-this-live answer is: the door decides where a session's state + // lives, the session carries its identity, and the store holds rows. A + // worker built from a copied config inherits the same key, so a task node's + // memory reading is scoped exactly as its parent's was. + MemoryProjectKey string + + // MemoryIndex is the store THIS SESSION'S CONVERSATION is indexed into, + // and it is deliberately not [Config.Memory]: with memory off, the session + // writes nothing about the person and still writes its own words where the + // next search can find them. Nil indexes nothing, which is a session built + // by a caller that opens no store at all. + // + // Every reader falls back to Memory when this is unset, so a door that + // never adopted the split behaves exactly as it did. + MemoryIndex *store.Store + // ApprovalPolicy decides whether a tool call runs, asks, or is refused // (internal/approval, gated in consent.go). NIL ALLOWS EVERYTHING, which is // the behavior every caller had before the gate existed: a headless --once diff --git a/internal/session/tools_conversations.go b/internal/session/tools_conversations.go index c6b5a29d09..3e286b2412 100644 --- a/internal/session/tools_conversations.go +++ b/internal/session/tools_conversations.go @@ -57,6 +57,11 @@ func parseConversationReference(ref string) (conversationReference, error) { } // conversationHistory preserves an explicit read-only source across descendants. +// +// THE ORDER IS THE DECOUPLING: an explicit reader wins, then the memory store +// (which every pre-split caller handed for both purposes), then the +// conversation index — so a session with memory OFF and an index still has +// the search verb. MEMORY OFF IS NOT SEARCH OFF; it is not remembering. func (c Config) conversationHistory() ConversationHistoryReader { if c.ConversationHistory != nil { return c.ConversationHistory @@ -64,9 +69,23 @@ func (c Config) conversationHistory() ConversationHistoryReader { if c.Memory != nil { return c.Memory } + if c.MemoryIndex != nil { + return c.MemoryIndex + } return nil } +// conversationIndexStore is the store this session's conversation is INDEXED +// into: the explicit index when the door opened one, else the memory store, +// which every door before the split passed for both purposes. Nil indexes +// nothing — and indexing nothing with memory off was the old defect. +func (c Config) conversationIndexStore() *store.Store { + if c.MemoryIndex != nil { + return c.MemoryIndex + } + return c.Memory +} + func (c Config) hasConversationHistory() bool { return c.conversationHistory() != nil } const ( diff --git a/internal/session/world.go b/internal/session/world.go index caae9ca891..a178e6cb1b 100644 --- a/internal/session/world.go +++ b/internal/session/world.go @@ -434,6 +434,45 @@ func ReadWorld(root string) World { // ReadHome is every project under this machine's state root. func ReadHome() World { return ReadWorld(PlacesRoot()) } +// SessionWorkspaces answers where every recorded session ran: the session id +// to its workspace, read off each folder's own meta.json — the one place a +// session's workspace is stated as a fact (the bucket name above it is an +// encoding, not an identity, and world.go's header says why it is one-way). +// +// It is the map the memory migration's re-home pass asks for: a quarantined +// row's source session, looked up here, is the authoritative answer to WHICH +// PROJECT this memory belonged to. A session whose meta cannot be read is +// simply absent — an unreadable folder is not an owner anybody can prove. +func SessionWorkspaces() map[string]string { + answered := map[string]string{} + root := PlacesRoot() + buckets, err := os.ReadDir(root) + if err != nil { + return answered + } + for _, bucket := range buckets { + if !bucket.IsDir() { + continue + } + sessions, err := os.ReadDir(filepath.Join(root, bucket.Name())) + if err != nil { + continue + } + for _, entry := range sessions { + if !entry.IsDir() { + continue + } + dir := filepath.Join(root, bucket.Name(), entry.Name()) + meta, err := LoadMeta(dir) + if err != nil || meta.ID == "" || strings.TrimSpace(meta.Workspace) == "" { + continue + } + answered[meta.ID] = meta.Workspace + } + } + return answered +} + // ReadRows is the rows of the named conversations and of nothing else, keyed by // each transcript's cleaned path: one [readSessionRow] per name, read exactly as // the walk reads that folder. diff --git a/internal/store/facts.go b/internal/store/facts.go index d058da918b..3fa2ef7017 100644 --- a/internal/store/facts.go +++ b/internal/store/facts.go @@ -2238,7 +2238,7 @@ func migrateFactsSchema(db *sql.DB) error { default: continue } - if err := replayEvent(tx, event); err != nil { + if err := replayEvent(tx, event, false); err != nil { return err } } diff --git a/internal/store/memory.go b/internal/store/memory.go index 7791fd3c19..ed352c2501 100644 --- a/internal/store/memory.go +++ b/internal/store/memory.go @@ -61,6 +61,19 @@ const ( // that carry none — so the journal takes a snapshot on an interval instead // and a replay lands on that floor rather than on nothing. EventMemoryRanking EventKind = "memory_ranking" + // EventMemorySkipped journals a write the dedup door refused. A skip is an + // outcome, not a silence: the person said "remember X" twice, and the + // record of both says so. It carries the text and the id it matched. + EventMemorySkipped EventKind = "memory_skipped" + // EventMemoryWriteFailed journals a write that did not land — an + // extraction the store refused, an import row that failed. Errors used to + // vanish at every one of these doors; now every failure that reaches this + // kind is readable at the store, and the session's dim line points here. + EventMemoryWriteFailed EventKind = "memory_write_failed" + // EventMemoryRehomed journals a quarantine row's move to a provable owner. + // A permission change is exactly the kind of thing the journal exists to + // carry: it can be replayed, audited, and — much later, with sync — folded. + EventMemoryRehomed EventKind = "memory_rehomed" ) // The five kinds of thing worth remembering across sessions. They are separate @@ -78,6 +91,11 @@ const ( // A memory's scope is the blast radius of its truth: something true about the // person everywhere, something true only inside this project, or something true // only of this machine. +// +// SCOPE IS DERIVED FROM OWNER NOW (memory_owner.go), and the column stays +// because the snapshot's shelf grouping and every row the migration already +// moved read it. A caller setting one without the other gets the owner's — +// the payload gate derives scope from owner and refuses the reverse drift. const ( MemoryScopeUser = "user" MemoryScopeProject = "project" @@ -108,7 +126,12 @@ const ( // Memory is one durable thing known across sessions. type Memory struct { - ID string + ID string + // Owner is WHO the memory belongs to — the whole of the permission model + // (memory_owner.go). Every read that can put a memory in front of a model + // filters on it; two projects may hold genuinely different truths about + // the same words, and never see each other's. + Owner string Type string Scope string Title string @@ -163,9 +186,17 @@ type MemoryStub struct{ ID, Title, Type, Scope string } // view and search" a property of the write rather than a filter every reader // has to remember — a superseded or forgotten row leaves the index at the // moment it stops being true. +// +// THE INDEX IS CREATED SEPARATELY FROM THE TABLE, and only when a probe proves +// FTS5 works in this build ([Store.fts]). It has always worked in the SQLite +// this module ships, and the probe is not a hedge against that — it is the +// answer to "what happens on a build where it does not": memory stays readable, +// writable and owner-scoped, and only the lexical tier goes quiet, the same way +// a store with no vectors answers without them. const memoriesSchema = ` CREATE TABLE IF NOT EXISTS memories ( id TEXT PRIMARY KEY, + owner TEXT NOT NULL DEFAULT 'user', type TEXT NOT NULL, scope TEXT NOT NULL, title TEXT NOT NULL, @@ -181,6 +212,10 @@ CREATE TABLE IF NOT EXISTS memories ( ); CREATE INDEX IF NOT EXISTS memories_status_updated ON memories (status, updated_seq DESC); CREATE INDEX IF NOT EXISTS memories_status_scope ON memories (status, scope, updated_seq DESC); +CREATE INDEX IF NOT EXISTS memories_owner_active ON memories (owner, updated_seq DESC) WHERE status = 'active'; +` + +const memoriesFTSSchema = ` CREATE VIRTUAL TABLE IF NOT EXISTS memories_fts USING fts5( memory_id UNINDEXED, title, @@ -210,6 +245,7 @@ func migrateMemoriesSchema(db *sql.DB) error { type memoryPayload struct { ID string `json:"id"` + Owner string `json:"owner,omitempty"` Type string `json:"type"` Scope string `json:"scope"` Title string `json:"title"` @@ -311,14 +347,14 @@ func (s *Store) addMemory(m Memory, sourceSession string) (Memory, error) { if err != nil { return Memory{}, fmt.Errorf("add memory: %w", err) } - if err := applyMemoryAdd(tx, payload, seq); err != nil { + if err := applyMemoryAdd(tx, payload, seq, s.fts); err != nil { return Memory{}, fmt.Errorf("add memory: %w", err) } if err := tx.Commit(); err != nil { return Memory{}, fmt.Errorf("add memory: %w", err) } return Memory{ - ID: payload.ID, Type: payload.Type, Scope: payload.Scope, + ID: payload.ID, Owner: payload.Owner, Type: payload.Type, Scope: payload.Scope, Title: payload.Title, Text: payload.Text, Tags: payload.Tags, Status: MemoryActive, CreatedSeq: seq, UpdatedSeq: seq, SourceSession: payload.SourceSession, SourceSeq: memorySourceSeq(payload.SourceSession, seq), @@ -329,6 +365,17 @@ func (s *Store) addMemory(m Memory, sourceSession string) (Memory, error) { // because a memory that changed either of those is a different memory and // wants SupersedeMemory: the whole point of an update is that the router's // existing pointers to this id stay valid. +// UpdateMemory, UpdateMemoryFromSession, SupersedeMemory, +// ForgetMemoryFromSession and RestoreMemory act BY ID, with no owner filter — +// that is a contract, not an oversight, and it is written here so the next +// caller reads it before it becomes a hole. A raw id door is for two callers +// only: the person's own surface (the memory place, which may manage any row +// it is showing), and a caller that has PROVEN the row's visibility first — +// the session's write path reads the target through its owner filter before +// it updates or supersedes, and its forget match is scoped the same way. A +// caller that holds an id from a model or from the wire and has not proven +// where it came from must not call these; it should go through reads that +// name owners, and write through [Store.Write]. func (s *Store) UpdateMemory(id, title, text string, tags []string) error { return s.UpdateMemoryFromSession(id, title, text, tags, "") } @@ -354,7 +401,7 @@ func (s *Store) UpdateMemoryFromSession(id, title, text string, tags []string, s if err != nil { return fmt.Errorf("update memory: %w", err) } - if err := applyMemoryUpdate(tx, payload, seq); err != nil { + if err := applyMemoryUpdate(tx, payload, seq, s.fts); err != nil { return fmt.Errorf("update memory: %w", err) } if err := tx.Commit(); err != nil { @@ -399,20 +446,143 @@ func (s *Store) SupersedeMemory(oldID string, m Memory) (Memory, error) { if err != nil { return Memory{}, fmt.Errorf("supersede memory: %w", err) } - if err := applyMemorySupersede(tx, payload, seq); err != nil { + if err := applyMemorySupersede(tx, payload, seq, s.fts); err != nil { return Memory{}, fmt.Errorf("supersede memory: %w", err) } if err := tx.Commit(); err != nil { return Memory{}, fmt.Errorf("supersede memory: %w", err) } return Memory{ - ID: fresh.ID, Type: fresh.Type, Scope: fresh.Scope, + ID: fresh.ID, Owner: fresh.Owner, Type: fresh.Type, Scope: fresh.Scope, Title: fresh.Title, Text: fresh.Text, Tags: fresh.Tags, Status: MemoryActive, CreatedSeq: seq, UpdatedSeq: seq, SourceSession: fresh.SourceSession, SourceSeq: memorySourceSeq(fresh.SourceSession, seq), }, nil } +// SupersedeMemoryForOwners retires one memory and admits its replacement, under +// the constraint that the old row is owned by one of the named owners. The +// owners list is the store-level enforcement: even a caller that holds a foreign +// id cannot retire another project's memory through this door. The raw-id door +// [Store.SupersedeMemory] remains for the person's own surface (which may +// manage any row it is showing) and for callers that have already proven +// visibility. +func (s *Store) SupersedeMemoryForOwners(owners []string, oldID string, m Memory) (Memory, error) { + if len(owners) == 0 { + return Memory{}, fmt.Errorf("supersede memory: %w: no owner was named", ErrInvalid) + } + oldID = strings.TrimSpace(oldID) + if oldID == "" { + return Memory{}, fmt.Errorf("supersede memory: %w: id is required", ErrInvalid) + } + fresh, err := memoryPayloadFrom(m) + if err != nil { + return Memory{}, fmt.Errorf("supersede memory: %w", err) + } + if fresh.ID == "" { + fresh.ID = NewMemoryID() + } + if fresh.ID == oldID { + return Memory{}, fmt.Errorf("supersede memory: %w: %q cannot supersede itself", ErrInvalid, oldID) + } + tx, err := s.beginWrite() + if err != nil { + return Memory{}, fmt.Errorf("supersede memory: %w", err) + } + defer tx.Rollback() + + // THE OLD ROW'S OWNER IS IN THE NAMED OWNERS, or the supersession is + // refused. This is the store-level guard the session's own visibility + // check already makes, and it is what closes the raw-id door: a decider + // naming (or a stale neighbor list carrying) another project's memory id + // cannot mark that row superseded here. + where, ownerArgs := ownerFilterSQL(owners) + var count int + if err := tx.QueryRow(`SELECT COUNT(*) FROM memories WHERE id = ? AND status = ?`+where, + append([]any{oldID, MemoryActive}, ownerArgs...)...).Scan(&count); err != nil { + return Memory{}, fmt.Errorf("supersede memory: %w", err) + } + if count == 0 { + return Memory{}, fmt.Errorf("supersede memory: %w: %q is not an active memory this session can see", ErrInvalid, oldID) + } + + var exists int + if err := tx.QueryRow(`SELECT COUNT(*) FROM memories WHERE id = ?`, fresh.ID).Scan(&exists); err != nil { + return Memory{}, fmt.Errorf("supersede memory: %w", err) + } + if exists != 0 { + return Memory{}, fmt.Errorf("supersede memory: %w: %q already exists", ErrInvalid, fresh.ID) + } + fresh.SourceSession = strings.TrimSpace(m.SourceSession) + payload := memorySupersedePayload{OldID: oldID, New: fresh, SourceSession: fresh.SourceSession} + seq, _, err := appendEvent(tx, fresh.ID, EventMemorySupersede, payload) + if err != nil { + return Memory{}, fmt.Errorf("supersede memory: %w", err) + } + if err := applyMemorySupersede(tx, payload, seq, s.fts); err != nil { + return Memory{}, fmt.Errorf("supersede memory: %w", err) + } + if err := tx.Commit(); err != nil { + return Memory{}, fmt.Errorf("supersede memory: %w", err) + } + return Memory{ + ID: fresh.ID, Owner: fresh.Owner, Type: fresh.Type, Scope: fresh.Scope, + Title: fresh.Title, Text: fresh.Text, Tags: fresh.Tags, + Status: MemoryActive, CreatedSeq: seq, UpdatedSeq: seq, + SourceSession: fresh.SourceSession, SourceSeq: memorySourceSeq(fresh.SourceSession, seq), + }, nil +} + +// UpdateMemoryForOwners updates a memory only when the target's owner is in the +// named owners list. It is the store-level guard: the session's own visibility +// check closes one door, and this closes the other. A caller that holds a +// foreign id from a stale neighbor list or a model invention cannot rewrite +// another project's memory through this door. +func (s *Store) UpdateMemoryForOwners(owners []string, id, title, text string, tags []string, sourceSession string) error { + if len(owners) == 0 { + return fmt.Errorf("update memory: %w: no owner was named", ErrInvalid) + } + id = strings.TrimSpace(id) + if id == "" { + return fmt.Errorf("update memory: %w: id is required", ErrInvalid) + } + title, text, tags, err := validMemoryBody(title, text, tags) + if err != nil { + return fmt.Errorf("update memory: %w", err) + } + tx, err := s.beginWrite() + if err != nil { + return fmt.Errorf("update memory: %w", err) + } + defer tx.Rollback() + + // THE TARGET ROW'S OWNER IS IN THE NAMED OWNERS, or the update is refused. + // A store error on this read also refuses: a guard that cannot prove the + // target is owned by this session must never become a pass. + where, ownerArgs := ownerFilterSQL(owners) + var count int + if err := tx.QueryRow(`SELECT COUNT(*) FROM memories WHERE id = ? AND status = ?`+where, + append([]any{id, MemoryActive}, ownerArgs...)...).Scan(&count); err != nil { + return fmt.Errorf("update memory: %w", err) + } + if count == 0 { + return fmt.Errorf("update memory: %w: %q is not an active memory this session can see", ErrInvalid, id) + } + + payload := memoryUpdatePayload{ID: id, Title: title, Text: text, Tags: tags, SourceSession: strings.TrimSpace(sourceSession)} + seq, _, err := appendEvent(tx, id, EventMemoryUpdate, payload) + if err != nil { + return fmt.Errorf("update memory: %w", err) + } + if err := applyMemoryUpdate(tx, payload, seq, s.fts); err != nil { + return fmt.Errorf("update memory: %w", err) + } + if err := tx.Commit(); err != nil { + return fmt.Errorf("update memory: %w", err) + } + return nil +} + // ForgetMemory tombstones a memory. It refuses a memory that is already // forgotten rather than returning quietly: "forget that" is an instruction a // person expects to have had an effect, and a silent success on a row that was @@ -438,7 +608,7 @@ func (s *Store) ForgetMemoryFromSession(id, sourceSession string) error { if err != nil { return fmt.Errorf("forget memory: %w", err) } - if err := applyMemoryForget(tx, payload, seq); err != nil { + if err := applyMemoryForget(tx, payload, seq, s.fts); err != nil { return fmt.Errorf("forget memory: %w", err) } if err := tx.Commit(); err != nil { @@ -464,7 +634,7 @@ func (s *Store) RestoreMemory(id string) error { if err != nil { return fmt.Errorf("restore memory: %w", err) } - if err := applyMemoryRestore(tx, payload, seq); err != nil { + if err := applyMemoryRestore(tx, payload, seq, s.fts); err != nil { return fmt.Errorf("restore memory: %w", err) } if err := tx.Commit(); err != nil { @@ -473,15 +643,22 @@ func (s *Store) RestoreMemory(id string) error { return nil } -// GetMemories reads full memories by id, in the order asked for. +// GetMemories reads full memories by id, in the order asked for, and returns +// only the ones the caller may SEE: owner-filtered and active-only. The owner +// filter is the point — a pointer to an id survives in transcripts, in a +// session's held lines and in a task's brief, and an id that names another +// project's memory must hand back nothing, not somebody else's truth. // // Input order is the contract because the caller is a router that has already // decided what matters most; re-sorting its choice by anything the store knows // would discard the one ranking in the system that saw the actual question. Ids -// that name nothing, or name something no longer active, are simply absent — -// a memory the user forgot between the index read and this call must not -// reappear because a pointer to it survived. -func (s *Store) GetMemories(ids []string) ([]Memory, error) { +// that name nothing, or name something no longer active or not the caller's, +// are simply absent — a memory the user forgot between the index read and this +// call must not reappear because a pointer to it survived. +func (s *Store) GetMemories(owners []string, ids []string) ([]Memory, error) { + if len(owners) == 0 { + return nil, fmt.Errorf("get memories: %w: no owner was named", ErrInvalid) + } wanted := make([]string, 0, len(ids)) seen := make(map[string]bool, len(ids)) for _, id := range ids { @@ -501,8 +678,10 @@ func (s *Store) GetMemories(ids []string) ([]Memory, error) { args = append(args, id) } args = append(args, MemoryActive) + where, ownerArgs := ownerFilterSQL(owners) + args = append(args, ownerArgs...) found, err := s.queryMemories( - `WHERE id IN (`+placeholders+`) AND status = ?`, args, "", 0) + `WHERE id IN (`+placeholders+`) AND status = ?`+where, args, "", 0) if err != nil { return nil, fmt.Errorf("get memories: %w", err) } @@ -531,15 +710,36 @@ func (s *Store) GetMemories(ids []string) ([]Memory, error) { // that safe to say: it reduces the caller's words to quoted alphanumeric terms // joined by OR, so hostile FTS syntax never reaches MATCH and a failure here is // a real failure worth returning. -func (s *Store) SearchMemories(query string, limit int) ([]Memory, error) { +// SearchMemories finds active memories the caller may see, by their words, best +// match first. The owners list is the search's blast radius: two projects may +// hold genuinely different truths about the same words, so a search never +// leaves the owners it was handed — dedup, forget-matching and person queries +// all go through it, and all of them are scoped the same way by construction. +// +// An empty owners list is an error and not "everything": this read feeds +// decisions a model acts on, and "unscoped" would be the scope leak the owner +// column exists to close. The janitor's unscoped read is [Store.ListMemories]. +// +// A store without the FTS index answers nil: the lexical tier is the whole of +// this read, and a missing tier is a miss, not a failure. +func (s *Store) SearchMemories(owners []string, query string, limit int) ([]Memory, error) { + if len(owners) == 0 { + return nil, fmt.Errorf("search memories: %w: no owner was named", ErrInvalid) + } + if !s.fts { + return nil, nil + } terms := ftsQueryFrom(query) if terms == "" { return nil, nil } + where, ownerArgs := ownerFilterSQL(owners) + args := []any{terms, MemoryActive} + args = append(args, ownerArgs...) memories, err := s.queryMemories(` JOIN memories_fts ON memories_fts.memory_id = memories.id - WHERE memories_fts MATCH ? AND memories.status = ?`, - []any{terms, MemoryActive}, + WHERE memories_fts MATCH ? AND memories.status = ?`+where, + args, `bm25(memories_fts), memories.updated_seq DESC`, limit) if err != nil { return nil, fmt.Errorf("search memories: %w", err) @@ -547,16 +747,19 @@ func (s *Store) SearchMemories(query string, limit int) ([]Memory, error) { return memories, nil } -// ListMemories returns active memories newest-touched first. An empty scope -// means every scope, which is what a person means by "what do you remember". -func (s *Store) ListMemories(scope string, limit int) ([]Memory, error) { - where := `WHERE status = ?` - args := []any{MemoryActive} - if scope = strings.TrimSpace(scope); scope != "" { - where += ` AND scope = ?` - args = append(args, scope) - } - memories, err := s.queryMemories(where, args, `updated_seq DESC, id`, limit) +// ListMemories returns active memories newest-touched first. +// +// THE OWNERS LIST IS THE ONLY FILTER, and an empty one is deliberate: this is +// the janitor's read — the background tidy, the demo seeder, a repair pass — +// and it is the one place "every owner" is an honest answer, because its work +// is by id against the row's own owner and never a retrieval. Every caller +// that can put a row in front of a model names its owners and the type system +// will not say so twice: [Store.MemoryCandidates] and [Store.SearchMemories] +// refuse the empty list. +func (s *Store) ListMemories(owners []string, limit int) ([]Memory, error) { + where, ownerArgs := ownerFilterSQL(owners) + args := append([]any{MemoryActive}, ownerArgs...) + memories, err := s.queryMemories(`WHERE status = ?`+where, args, `updated_seq DESC, id`, limit) if err != nil { return nil, fmt.Errorf("list memories: %w", err) } @@ -576,14 +779,15 @@ func (s *Store) ListMemories(scope string, limit int) ([]Memory, error) { // It is a separate read from ListMemories rather than a projection of it // because carrying five hundred bodies to render five hundred titles is how a // memory store becomes the most expensive thing in the loop. -func (s *Store) MemoryIndex(limit int) ([]MemoryStub, error) { +func (s *Store) MemoryIndex(owners []string, limit int) ([]MemoryStub, error) { // THE INDEX IS A RANKING, NOT A CHRONOLOGY. Memories that have proved useful // lead; updated sequence only settles equal-use rows. + where, ownerArgs := ownerFilterSQL(owners) statement := ` SELECT id, title, type, scope FROM memories - WHERE status = ? + WHERE status = ?` + where + ` ORDER BY use_count DESC, updated_seq DESC, id` - args := []any{MemoryActive} + args := append([]any{MemoryActive}, ownerArgs...) if limit > 0 { statement += ` LIMIT ?` args = append(args, limit) @@ -669,7 +873,10 @@ const rrfK = 60 // shorter than the index keeps — is not an error and not an empty answer: the // two arithmetic orderings still rank, so a person who types "ok, do it" is // still shown what has mattered most and what changed last. -func (s *Store) MemoryCandidates(terms string, limit int) ([]MemoryStub, error) { +func (s *Store) MemoryCandidates(owners []string, terms string, limit int) ([]MemoryStub, error) { + if len(owners) == 0 { + return nil, fmt.Errorf("memory candidates: %w: no owner was named", ErrInvalid) + } if limit <= 0 { limit = MemoryCandidatesDefault } @@ -677,8 +884,10 @@ func (s *Store) MemoryCandidates(terms string, limit int) ([]MemoryStub, error) // lexical ranking is left out of the fusion entirely when there is nothing // to match with. The other two still rank every row. lexical := `SELECT '' AS id, 0 AS rank WHERE 0` + where, ownerArgs := ownerFilterSQL(owners) args := []any{MemoryActive} - if query := ftsQueryFrom(terms); query != "" { + args = append(args, ownerArgs...) + if query := ftsQueryFrom(terms); query != "" && s.fts { lexical = ` SELECT id, ROW_NUMBER() OVER (ORDER BY relevance, updated_seq DESC, id) AS rank FROM ( @@ -692,7 +901,7 @@ func (s *Store) MemoryCandidates(terms string, limit int) ([]MemoryStub, error) statement := ` WITH active AS ( SELECT id, title, type, scope, use_count, miss_count, updated_seq - FROM memories WHERE status = ? + FROM memories WHERE status = ?` + where + ` ), lexical AS (` + lexical + `), important AS ( @@ -935,7 +1144,7 @@ func queryMemoriesOn(db memoryQuerier, where string, args []any, order string, l // that rewrote it would be a reader with two shapes. One statement, one // rowid seek per row carried out, and no second read from Go. statement := ` - SELECT memories.id, memories.type, memories.scope, memories.title, + SELECT memories.id, memories.owner, memories.type, memories.scope, memories.title, memories.text, memories.tags, memories.status, memories.use_count, memories.miss_count, memories.created_seq, memories.updated_seq, memories.source_session, memories.source_seq, @@ -957,7 +1166,7 @@ func queryMemoriesOn(db memoryQuerier, where string, args []any, order string, l for rows.Next() { var memory Memory var tags, updatedAt string - if err := rows.Scan(&memory.ID, &memory.Type, &memory.Scope, &memory.Title, + if err := rows.Scan(&memory.ID, &memory.Owner, &memory.Type, &memory.Scope, &memory.Title, &memory.Text, &tags, &memory.Status, &memory.UseCount, &memory.MissCount, &memory.CreatedSeq, &memory.UpdatedSeq, &memory.SourceSession, &memory.SourceSeq, &updatedAt); err != nil { @@ -984,22 +1193,28 @@ func queryMemoriesOn(db memoryQuerier, where string, args []any, order string, l // applyMemoryAdd materializes one added memory. It is shared by the write path // and by Rebuild, so a replayed brain and a live one cannot differ. -func applyMemoryAdd(tx *sql.Tx, payload memoryPayload, seq int64) error { +func applyMemoryAdd(tx *sql.Tx, payload memoryPayload, seq int64, fts bool) error { + // A PAYLOAD WITH NO OWNER IS A JOURNAL WRITTEN BEFORE THE COLUMN EXISTED, + // and it is answered the way the migration answers its rows: from the scope + // the row carries, with a bare `project` quarantined. An insert here must + // never land an empty owner — the column is NOT NULL and an empty owner in + // a view would be a row nothing can see and nothing can prove. + payload.Owner = ownerForReplay(payload) tags, err := memoryTagsJSON(payload.Tags) if err != nil { return err } if _, err := tx.Exec(` - INSERT INTO memories (id, type, scope, title, text, tags, status, use_count, created_seq, updated_seq, source_session, source_seq) - VALUES (?, ?, ?, ?, ?, ?, ?, 0, ?, ?, ?, ?)`, - payload.ID, payload.Type, payload.Scope, payload.Title, payload.Text, + INSERT INTO memories (id, owner, type, scope, title, text, tags, status, use_count, created_seq, updated_seq, source_session, source_seq) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, 0, ?, ?, ?, ?)`, + payload.ID, payload.Owner, payload.Type, payload.Scope, payload.Title, payload.Text, tags, MemoryActive, seq, seq, payload.SourceSession, memorySourceSeq(payload.SourceSession, seq)); err != nil { return err } - return refreshMemoryFTS(tx, payload.ID) + return refreshMemoryFTS(tx, payload.ID, fts) } -func applyMemoryUpdate(tx *sql.Tx, payload memoryUpdatePayload, seq int64) error { +func applyMemoryUpdate(tx *sql.Tx, payload memoryUpdatePayload, seq int64, fts bool) error { tags, err := memoryTagsJSON(payload.Tags) if err != nil { return err @@ -1018,10 +1233,10 @@ func applyMemoryUpdate(tx *sql.Tx, payload memoryUpdatePayload, seq int64) error if changed != 1 { return fmt.Errorf("%w: update targets missing or inactive memory %q", ErrInvalid, payload.ID) } - return refreshMemoryFTS(tx, payload.ID) + return refreshMemoryFTS(tx, payload.ID, fts) } -func applyMemorySupersede(tx *sql.Tx, payload memorySupersedePayload, seq int64) error { +func applyMemorySupersede(tx *sql.Tx, payload memorySupersedePayload, seq int64, fts bool) error { result, err := tx.Exec(` UPDATE memories SET status = ?, updated_seq = ?, source_session = ?, source_seq = ? WHERE id = ? AND status = ?`, @@ -1036,13 +1251,13 @@ func applyMemorySupersede(tx *sql.Tx, payload memorySupersedePayload, seq int64) if changed != 1 { return fmt.Errorf("%w: supersession targets missing or inactive memory %q", ErrInvalid, payload.OldID) } - if err := refreshMemoryFTS(tx, payload.OldID); err != nil { + if err := refreshMemoryFTS(tx, payload.OldID, fts); err != nil { return err } - return applyMemoryAdd(tx, payload.New, seq) + return applyMemoryAdd(tx, payload.New, seq, fts) } -func applyMemoryForget(tx *sql.Tx, payload memoryForgetPayload, seq int64) error { +func applyMemoryForget(tx *sql.Tx, payload memoryForgetPayload, seq int64, fts bool) error { result, err := tx.Exec(` UPDATE memories SET status = ?, updated_seq = ?, source_session = ?, source_seq = ? WHERE id = ? AND status <> ?`, @@ -1057,7 +1272,7 @@ func applyMemoryForget(tx *sql.Tx, payload memoryForgetPayload, seq int64) error if changed != 1 { return fmt.Errorf("%w: nothing to forget under %q", ErrInvalid, payload.ID) } - return refreshMemoryFTS(tx, payload.ID) + return refreshMemoryFTS(tx, payload.ID, fts) } // applyMemoryRanking restores the counters one snapshot recorded. It is shared @@ -1089,7 +1304,7 @@ func memorySourceSeq(sourceSession string, seq int64) int64 { return seq } -func applyMemoryRestore(tx *sql.Tx, payload memoryRestorePayload, seq int64) error { +func applyMemoryRestore(tx *sql.Tx, payload memoryRestorePayload, seq int64, fts bool) error { result, err := tx.Exec(`UPDATE memories SET status = ?, updated_seq = ? WHERE id = ? AND status = ?`, MemoryActive, seq, payload.ID, MemoryForgotten) if err != nil { @@ -1102,7 +1317,7 @@ func applyMemoryRestore(tx *sql.Tx, payload memoryRestorePayload, seq int64) err if changed != 1 { return fmt.Errorf("%w: restore targets missing or non-forgotten memory %q", ErrInvalid, payload.ID) } - return refreshMemoryFTS(tx, payload.ID) + return refreshMemoryFTS(tx, payload.ID, fts) } // memoryTagsJSON is why a memory with no tags reads back the same way after a @@ -1133,7 +1348,13 @@ func memoryTagsJSON(tags []string) (string, error) { // superseded or forgotten row matches nothing, so the DELETE stands and the row // is out of every search from that instant — no reader has to remember to // filter it, and none can forget. -func refreshMemoryFTS(tx *sql.Tx, id string) error { +func refreshMemoryFTS(tx *sql.Tx, id string, fts bool) error { + if !fts { + // NO INDEX, NOTHING TO REFRESH — and nothing to get wrong: a store + // running without FTS5 must not have its writes fail because the index + // is not there. + return nil + } if _, err := tx.Exec(`DELETE FROM memories_fts WHERE memory_id = ?`, id); err != nil { return err } @@ -1145,21 +1366,52 @@ func refreshMemoryFTS(tx *sql.Tx, id string) error { } // memoryPayloadFrom is the one gate every written memory passes through. +// +// OWNER IS THE AUTHORITY AND SCOPE IS DERIVED from it (memory_owner.go): a +// caller that names both gets the owner's scope, and a caller that names only +// the old scope word is answered as a legacy write — `user` and `env` map to +// their owners, and a bare `project` maps to the QUARANTINE, because a caller +// that cannot name the project's key has just demonstrated that it cannot prove +// the row belongs anywhere. The old `MemoryScopeProject` constant stays part of +// the surface for the snapshot's shelf words; the write path never trusts it as +// an identity. func memoryPayloadFrom(m Memory) (memoryPayload, error) { memoryType := strings.ToLower(strings.TrimSpace(m.Type)) if !validMemoryType(memoryType) { return memoryPayload{}, fmt.Errorf("%w: unknown memory type %q", ErrInvalid, m.Type) } - scope := strings.ToLower(strings.TrimSpace(m.Scope)) - if !validMemoryScope(scope) { - return memoryPayload{}, fmt.Errorf("%w: unknown memory scope %q", ErrInvalid, m.Scope) + owner := normalizeOwner(m.Owner) + if owner == "" { + // THE LEGACY CALLER. Only Scope was set. user and env are provable + // without anything else; a project is not, and lands in quarantine. + switch strings.ToLower(strings.TrimSpace(m.Scope)) { + case MemoryScopeUser: + owner = OwnerUser + case MemoryScopeEnv: + owner = OwnerMachine + case MemoryScopeProject: + owner = OwnerLegacyProject + default: + return memoryPayload{}, fmt.Errorf("%w: unknown memory scope %q", ErrInvalid, m.Scope) + } + } + if !ValidOwner(owner) { + return memoryPayload{}, fmt.Errorf("%w: unknown memory owner %q", ErrInvalid, m.Owner) } + scope := OwnerScopeOf(owner) title, text, tags, err := validMemoryBody(m.Title, m.Text, m.Tags) if err != nil { return memoryPayload{}, err } + if owner == OwnerLegacyProject && !hasTag(tags, legacyTag) { + // THE QUARANTINE CARRIES ITS MARK. A row nobody can prove the owner of + // is labeled as one, wherever it is read back — the person who is asked + // to re-home it deserves to see that this is what it is. + tags = append(tags, legacyTag) + } return memoryPayload{ ID: strings.TrimSpace(m.ID), + Owner: owner, Type: memoryType, Scope: scope, Title: title, diff --git a/internal/store/memory_owner.go b/internal/store/memory_owner.go new file mode 100644 index 0000000000..20b1f9546f --- /dev/null +++ b/internal/store/memory_owner.go @@ -0,0 +1,400 @@ +package store + +import ( + "context" + "database/sql" + "encoding/json" + "fmt" + "strings" +) + +// Owners are who a memory belongs to, and they are the whole of the +// permission model: a memory has exactly one owner, and every read that can +// put a memory in front of a model filters on owner equality. There is no +// audience list and no visibility knob to forget — sharing is a later, explicit +// copy with a new owner. +// +// The kinds a build mints today: +// +// OwnerUser the person, every project, every machine the store reaches +// OwnerMachine this machine only — an env-scoped memory's owner +// OwnerProject(key) one project; the key is [gitidentity.ProjectKey]'s output +// OwnerLegacyProject scope=project rows whose project nobody can prove +// +// OwnerLegacyProject is the quarantine. The old schema kept a `project` scope +// with no project identity on the row, so the true owner is unknowable from +// the row alone — and attributing one project's memory to the person at large, +// or to a guessed project, would widen what can see it. Quarantined rows are +// never injected, never deduplicated against, and never matched by a forget +// query; the person re-homes or lets go of them from the memory place, and a +// door that can prove a row's source session lived in a workspace can re-home +// it to that project ([Store.RehomeMemories]). +// +// team: owners are deliberately ABSENT, not deferred in a comment: no door +// mints one yet, and a capability that cannot work is absent rather than +// broken. Sharing with a team arrives with the sync work, as an explicit copy. +const ( + OwnerUser = "user" + OwnerMachine = "machine" + OwnerLegacyProject = "project:legacy" + + // legacyTag marks a row the migration quarantined. It survives re-homing + // on purpose: a memory's source is never cleaned off it, and a person who + // wants to know which rows came in blind can search for the tag. + legacyTag = "legacy-project" +) + +// OwnerProject spells a project owner from a project key. The key itself is +// opaque — a hash minted by internal/gitidentity — and is never a path: a path +// moves, a hash of a normalized remote does not. +func OwnerProject(key string) string { + return "project:" + strings.TrimSpace(key) +} + +// OwnerKindOf answers the kind word of an owner: `user`, `machine`, `project` +// or "" for an owner this build does not understand. +func OwnerKindOf(owner string) string { + switch { + case owner == OwnerUser || owner == OwnerMachine: + return owner + case strings.HasPrefix(owner, "project:"): + if key := strings.TrimPrefix(owner, "project:"); key != "" { + return "project" + } + } + return "" +} + +// OwnerScopeOf answers the legacy scope word a row's owner spells, which is +// what the `scope` column and the snapshot's shelf grouping keep reading. The +// mapping is fixed by the column's history: `user`, `project` and `env` are +// the three words the schema has always carried, and this is where owner and +// scope are kept from drifting apart — a write derives one from the other. +func OwnerScopeOf(owner string) string { + switch OwnerKindOf(owner) { + case OwnerUser: + return MemoryScopeUser + case OwnerMachine: + return MemoryScopeEnv + case "project": + return MemoryScopeProject + } + return MemoryScopeUser +} + +// ValidOwner answers whether owner is one this build can write. An empty owner +// is not valid here — every caller that has one names it, and the fallback for +// an empty owner lives at the call sites that know what an empty MEANS, not in +// the validator where it would silently widen. +func ValidOwner(owner string) bool { + return OwnerKindOf(owner) != "" +} + +// OwnerLegacyTag is the marker a quarantined row carries, for the reader that +// names it. +const OwnerLegacyTag = legacyTag + +// normalizeOwner is the one place an owner is cleaned before it is compared or +// written: trimmed, and matched as invalid when it is empty. +func normalizeOwner(owner string) string { + return strings.TrimSpace(owner) +} + +// ownerFilterSQL builds the owner clause the owner-keyed reads share. An empty +// owners list means "no filter" and is the janitor's answer only — the callers +// that must never see across owners assert the list is non-empty before they +// call, and this function is not where that is decided. +func ownerFilterSQL(owners []string) (string, []any) { + if len(owners) == 0 { + return "", nil + } + cleaned := make([]string, 0, len(owners)) + for _, owner := range owners { + if owner = normalizeOwner(owner); ValidOwner(owner) { + cleaned = append(cleaned, owner) + } + } + if len(cleaned) == 0 { + // EVERY OWNER NAMED WAS INVALID. That is a caller bug, and the honest + // answer is "nothing visible" rather than "everything visible": a + // filter that cannot be understood must never degrade to no filter. + return ` AND 1 = 0`, nil + } + placeholders := strings.TrimSuffix(strings.Repeat("?,", len(cleaned)), ",") + args := make([]any, 0, len(cleaned)) + for _, owner := range cleaned { + args = append(args, owner) + } + return ` AND memories.owner IN (` + placeholders + `)`, args +} + +// migrateMemoriesOwner gives every memory row an owner, once, from the only +// authority the old rows carry: +// +// - scope=user → owner=user (already the person's; nothing moves) +// - scope=env → owner=machine (this machine's own truth, renamed) +// - scope=project → owner=project:legacy, tagged legacy-project +// +// THE PROJECT ROWS ARE THE POINT AND THEY ARE NEVER GUESSED. The old scope +// said "true in a project" and carried no project, so the only honest owners +// are "this machine" (never widens) and "unknown" (never injects). Rows whose +// source session is later provable to a workspace are re-homed by the door +// that can do the proving ([Store.RehomeMemories]); everything else stays in +// quarantine, visible to the person and to nobody's model. +// +// It is idempotent — a row that already has an owner is left alone — and it is +// written in one transaction with the column it backfills. +func migrateMemoriesOwner(db *sql.DB) error { + found, err := tableHasColumn(db, "memories", "owner") + if err != nil { + return err + } + if found { + return nil + } + tx, err := db.BeginTx(context.Background(), nil) + if err != nil { + return fmt.Errorf("migrate memory owners: %w", err) + } + defer tx.Rollback() + + if _, err := tx.Exec(`ALTER TABLE memories ADD COLUMN owner TEXT NOT NULL DEFAULT ''`); err != nil { + return fmt.Errorf("migrate memory owners: %w", err) + } + // The column lands empty and is filled from scope in one pass each, so no + // row is ever left with an owner the validator would refuse. + if _, err := tx.Exec(`UPDATE memories SET owner = ? WHERE scope = ? AND owner = ''`, + OwnerUser, MemoryScopeUser); err != nil { + return fmt.Errorf("migrate memory owners: %w", err) + } + if _, err := tx.Exec(`UPDATE memories SET owner = ? WHERE scope = ? AND owner = ''`, + OwnerMachine, MemoryScopeEnv); err != nil { + return fmt.Errorf("migrate memory owners: %w", err) + } + rows, err := tx.Query(`SELECT id, tags FROM memories WHERE scope = ? AND owner = ''`, + MemoryScopeProject) + if err != nil { + return fmt.Errorf("migrate memory owners: %w", err) + } + type quarantined struct { + id string + tags []string + } + var pending []quarantined + for rows.Next() { + var id, tags string + if err := rows.Scan(&id, &tags); err != nil { + rows.Close() + return fmt.Errorf("migrate memory owners: %w", err) + } + var parsed []string + if err := json.Unmarshal([]byte(tags), &parsed); err != nil { + parsed = nil + } + pending = append(pending, quarantined{id: id, tags: parsed}) + } + if err := rows.Err(); err != nil { + rows.Close() + return fmt.Errorf("migrate memory owners: %w", err) + } + rows.Close() + for _, row := range pending { + if !hasTag(row.tags, legacyTag) { + row.tags = append(row.tags, legacyTag) + } + encoded, err := json.Marshal(row.tags) + if err != nil { + return fmt.Errorf("migrate memory owners: %w", err) + } + if _, err := tx.Exec(`UPDATE memories SET owner = ?, tags = ? WHERE id = ?`, + OwnerLegacyProject, string(encoded), row.id); err != nil { + return fmt.Errorf("migrate memory owners: %w", err) + } + } + // Anything still empty is a scope this build does not know, written by a + // later or a hand-edited store. It is quarantined too — the same answer, + // for the same reason: an owner nobody can prove never injects. + if _, err := tx.Exec(`UPDATE memories SET owner = ? WHERE owner = ''`, OwnerLegacyProject); err != nil { + return fmt.Errorf("migrate memory owners: %w", err) + } + if _, err := tx.Exec(`CREATE INDEX IF NOT EXISTS memories_owner_active ON memories (owner, updated_seq DESC) WHERE status = 'active'`); err != nil { + return fmt.Errorf("migrate memory owners: %w", err) + } + if err := tx.Commit(); err != nil { + return fmt.Errorf("migrate memory owners: %w", err) + } + return nil +} + +// RehomeMemories moves quarantined rows to a provable owner. It is the one +// door out of quarantine, and it takes ids rather than a scope: a pass that +// re-homes EVERYTHING quarantined to one project would be exactly the guess +// the migration refused to make. +// +// Only rows that ARE quarantined move — an id naming a row in any other owner +// is left untouched, because a re-home that could strip an owner it disagrees +// with would be a second, wider permission model hiding in a janitor's name. +func (s *Store) RehomeMemories(ids []string, owner string) (int, error) { + owner = normalizeOwner(owner) + if !ValidOwner(owner) { + return 0, fmt.Errorf("rehome memories: %w: owner %q is not one this build mints", ErrInvalid, owner) + } + if len(ids) == 0 { + return 0, nil + } + tx, err := s.beginWrite() + if err != nil { + return 0, fmt.Errorf("rehome memories: %w", err) + } + defer tx.Rollback() + moved := 0 + for _, id := range ids { + id = strings.TrimSpace(id) + if id == "" { + continue + } + result, err := tx.Exec(`UPDATE memories SET owner = ?, scope = ? WHERE id = ? AND owner = ?`, + owner, OwnerScopeOf(owner), id, OwnerLegacyProject) + if err != nil { + return 0, fmt.Errorf("rehome memories: %w", err) + } + changed, err := result.RowsAffected() + if err != nil { + return 0, fmt.Errorf("rehome memories: %w", err) + } + moved += int(changed) + } + // A re-home is a permission change and it is journaled as one: the event + // says which rows moved and where, so a later sync fold can agree about it. + if moved > 0 { + if _, _, err := appendEvent(tx, "memory", EventMemoryRehomed, map[string]any{ + "ids": ids, "owner": owner, + }); err != nil { + return 0, fmt.Errorf("rehome memories: %w", err) + } + } + if err := tx.Commit(); err != nil { + return 0, fmt.Errorf("rehome memories: %w", err) + } + return moved, nil +} + +// QuarantinedMemories lists the rows still in quarantine, whatever their +// status, so the one surface that offers re-homing can show them. This is a +// person's audit read and never a model's: nothing here is injected anywhere. +func (s *Store) QuarantinedMemories(limit int) ([]Memory, error) { + memories, err := s.queryMemories(`WHERE owner = ?`, []any{OwnerLegacyProject}, `updated_seq DESC, id`, limit) + if err != nil { + return nil, fmt.Errorf("quarantined memories: %w", err) + } + return memories, nil +} + +// ListProjectMemories lists every project-owned row — every project's rows +// AND the quarantine's — for the person's own project shelf. It is the read +// the surface's `project` scope word means: a person auditing their memory +// sees all of their projects, because "which project is this surface in" is +// not a question an audit page answers (a session's retrieval answers that, +// scoped to its own project, and the session reads are where the scoping +// lives). The quarantine's rows are here too: they are project rows whose +// project nobody proved, and hiding them from the person would be hiding the +// one place re-homing starts from. +func (s *Store) ListProjectMemories(limit int) ([]Memory, error) { + memories, err := s.queryMemories(`WHERE owner LIKE ?`, []any{"project:%"}, `updated_seq DESC, id`, limit) + if err != nil { + return nil, fmt.Errorf("list project memories: %w", err) + } + return memories, nil +} + +func hasTag(tags []string, want string) bool { + for _, tag := range tags { + if tag == want { + return true + } + } + return false +} + +// applyRehomeOne moves one id out of quarantine to owner, and answers whether +// it moved. It is the one place the fold and the replay both re-home through, +// so a remote fold and a journal replay cannot disagree about what a rehomed +// event means. A row is moved ONLY while its current owner is the quarantine — +// whatever its status, matching the live door, which re-homes forgotten rows +// too ([QuarantinedMemories] lists them whatever their status) — and the +// journal sequence stamps the row's updated_seq so the move is visible to +// pagination like any other touch. +func applyRehomeOne(tx *sql.Tx, id, owner string, seq int64) (bool, error) { + id = strings.TrimSpace(id) + if id == "" { + return false, nil + } + result, err := tx.Exec(`UPDATE memories SET owner = ?, scope = ?, updated_seq = ? + WHERE id = ? AND owner = ?`, + owner, OwnerScopeOf(owner), seq, id, OwnerLegacyProject) + if err != nil { + return false, err + } + changed, err := result.RowsAffected() + if err != nil { + return false, err + } + return changed == 1, nil +} + +// ownerForReplay answers the owner an event payload carries, or derives it the +// way the migration derives one from a bare scope word. It is the replay +// path's answer to "what did this row belong to before owners existed", and it +// is the same rule [memoryPayloadFrom] uses so a replayed row and a live row +// cannot disagree. +func ownerForReplay(payload memoryPayload) string { + if owner := normalizeOwner(payload.Owner); ValidOwner(owner) { + return owner + } + // An explicit owner that is not known to this build (e.g. a future team:… + // owner from a newer paired device) is QUARANTINED, never widened to user. + // A row whose owner cannot be understood must never degrade to one that + // is visible from every project; quarantined rows are never injected. + if owner := normalizeOwner(payload.Owner); owner != "" && !ValidOwner(owner) { + return OwnerLegacyProject + } + switch strings.ToLower(strings.TrimSpace(payload.Scope)) { + case MemoryScopeUser: + return OwnerUser + case MemoryScopeEnv: + return OwnerMachine + case MemoryScopeProject: + return OwnerLegacyProject + default: + // A scope word this build does not understand is quarantined: a filter + // that cannot be understood must never degrade to no filter. The caller + // that knows the correct owner overrides with its own payload.Owner. + return OwnerLegacyProject + } +} + +// applyMemoryRehomed reproduces a re-home during replay. It moves an id ONLY +// while that id is still quarantined, exactly what the live door would have +// moved: a re-home that could strip an owner it disagrees with would be a +// second, wider permission model hiding in a replay helper. The restriction is +// also what makes replay exact — the live door only ever moved quarantined +// rows, so a journal's rehomed events replay onto rows that are in quarantine +// when the event reaches them. +func applyMemoryRehomed(tx *sql.Tx, raw json.RawMessage, seq int64) error { + var payload struct { + IDs []string `json:"ids"` + Owner string `json:"owner"` + } + if err := json.Unmarshal(raw, &payload); err != nil { + return err + } + if !ValidOwner(payload.Owner) { + return fmt.Errorf("%w: rehomed event names owner %q", ErrInvalid, payload.Owner) + } + for _, id := range payload.IDs { + if _, err := applyRehomeOne(tx, id, payload.Owner, seq); err != nil { + return err + } + } + return nil +} diff --git a/internal/store/memory_owner_test.go b/internal/store/memory_owner_test.go new file mode 100644 index 0000000000..dfce03d3d4 --- /dev/null +++ b/internal/store/memory_owner_test.go @@ -0,0 +1,627 @@ +package store + +import ( + "encoding/json" + "errors" + "fmt" + "path/filepath" + "reflect" + "strings" + "testing" +) + +// ── owner isolation ───────────────────────────────────────────────────────── + +// THE OWNER IS THE PERMISSION MODEL. A memory written under one project's +// owner must be invisible to every read that names another: no injection, no +// dedup answer, no forget match. Two projects may hold genuinely different +// truths about the same words, and the reads are what keep them apart. +func TestAMemoryWrittenUnderOneOwnerIsInvisibleToAnother(t *testing.T) { + graph := openTestStore(t, filepath.Join(t.TempDir(), "isolation.db")) + alpha := OwnerProject("alpha") + beta := OwnerProject("beta") + + alphaRow, err := graph.Write(WriteRequest{ + Owner: alpha, Type: MemoryDecision, + Title: "Deploys to the amber cluster", Text: "Deploys to the amber cluster every Tuesday.", + SourceSession: "session-a", + }) + if err != nil || alphaRow.Outcome != WriteOutcomeAdded { + t.Fatalf("write into alpha = (%+v, %v)", alphaRow, err) + } + + // The router's shortlist under beta has nothing from alpha. + pool, err := graph.MemoryCandidates([]string{beta}, "amber cluster deploy", MemoryCandidatesDefault) + if err != nil { + t.Fatalf("MemoryCandidates under beta: %v", err) + } + if containsMemoryStub(pool, alphaRow.Memory.ID) { + t.Errorf("alpha's memory is in beta's pool: %+v", pool) + } + // A search under beta finds nothing of alpha's. + hits, err := graph.SearchMemories([]string{beta}, "amber cluster", 5) + if err != nil || len(hits) != 0 { + t.Fatalf("search under beta = (%v, %v), want nothing of alpha's", memoryIDs(hits), err) + } + // A get by id — a pointer that survived in a transcript — hands back nothing. + got, err := graph.GetMemories([]string{beta}, []string{alphaRow.Memory.ID}) + if err != nil || len(got) != 0 { + t.Fatalf("GetMemories under beta = (%v, %v), want the pointer refused", memoryIDs(got), err) + } + // And the owner who wrote it still sees it. + theirs, err := graph.SearchMemories([]string{alpha}, "amber cluster", 5) + if err != nil || len(theirs) != 1 || theirs[0].ID != alphaRow.Memory.ID { + t.Fatalf("search under alpha = (%v, %v), want its own row", memoryIDs(theirs), err) + } +} + +// An empty owners list is refused on every read that can put a memory in front +// of a model. "Unscoped" was the scope leak; the refusal is what keeps it +// closed even for a caller that forgot to name its owner. +func TestOwnerReadsRefuseAnEmptyOwnersList(t *testing.T) { + graph := openTestStore(t, filepath.Join(t.TempDir(), "unscoped.db")) + mustAddMemory(t, graph, Memory{Owner: OwnerUser, Type: MemoryFact, Title: "Fine", Text: "Fine."}) + for name, call := range map[string]func() error{ + "MemoryCandidates": func() error { _, err := graph.MemoryCandidates(nil, "fine", 8); return err }, + "SearchMemories": func() error { _, err := graph.SearchMemories(nil, "fine", 8); return err }, + "GetMemories": func() error { _, err := graph.GetMemories(nil, []string{"mem_x"}); return err }, + "MemoryPage": func() error { _, _, err := graph.MemoryPage(nil, MemoryCursor{}, 8); return err }, + } { + if err := call(); !errors.Is(err, ErrInvalid) { + t.Errorf("%s accepted an empty owners list: %v", name, err) + } + } + // ListMemories stays the janitor's read: empty means every owner, by + // contract, because its work is by id and never a retrieval. + all, err := graph.ListMemories(nil, 0) + if err != nil || len(all) != 1 { + t.Fatalf("the janitor's list = (%v, %v), want the row", memoryIDs(all), err) + } +} + +// The write door refuses an owner it cannot prove, and it is the reason the +// quarantine exists: a caller that cannot NAME a project cannot put a row into +// one. +func TestWriteRefusesAnOwnerItCannotProve(t *testing.T) { + graph := openTestStore(t, filepath.Join(t.TempDir(), "refuse.db")) + for name, req := range map[string]WriteRequest{ + "empty owner": {Type: MemoryFact, Title: "T", Text: "Body."}, + "vague owner": {Owner: "project", Type: MemoryFact, Title: "T", Text: "Body."}, + "junk owner": {Owner: "project:", Type: MemoryFact, Title: "T", Text: "Body."}, + "made-up kind": {Owner: "team:the-fold", Type: MemoryFact, Title: "T", Text: "Body."}, + } { + if _, err := graph.Write(req); !errors.Is(err, ErrInvalid) { + t.Errorf("Write(%s) = %v, want an ErrInvalid refusal", name, err) + } + } + // Nothing was written. + rows, err := graph.ListMemories(nil, 0) + if err != nil || len(rows) != 0 { + t.Fatalf("rows after refusals = (%v, %v), want an empty store", memoryIDs(rows), err) + } +} + +// ── the one write door ────────────────────────────────────────────────────── + +// A repeated "remember this" through the one door skips the second: exact +// words, or the same title, within the same owner, and the skip is journaled. +func TestWriteDoorSkipsADuplicateWithinTheSameOwner(t *testing.T) { + graph := openTestStore(t, filepath.Join(t.TempDir(), "door.db")) + alpha := OwnerProject("alpha") + + first, err := graph.Write(WriteRequest{Owner: alpha, Type: MemoryPreference, + Title: "Prefers tabs", Text: "Prefers tabs over spaces in Go files."}) + if err != nil || first.Outcome != WriteOutcomeAdded { + t.Fatalf("first write = (%+v, %v)", first, err) + } + // THE SAME WORDS, RETYPED — different case and spacing are the same words. + again, err := graph.Write(WriteRequest{Owner: alpha, Type: MemoryPreference, + Title: "Prefers tabs", Text: "PREFERS tabs over spaces in Go files."}) + if err != nil || again.Outcome != WriteOutcomeSkipped { + t.Fatalf("repeated write = (%+v, %v), want a skip", again, err) + } + if again.Memory.ID != first.Memory.ID { + t.Fatalf("skip names %q, want the row it matched %q", again.Memory.ID, first.Memory.ID) + } + // AND THE SAME TITLE IS ENOUGH — the title is the index line the router + // picks by, and two rows with one title would be two entries in every + // shortlist. + retitled, err := graph.Write(WriteRequest{Owner: alpha, Type: MemoryFact, + Title: "prefers TABS", Text: "An entirely different sentence about editors."}) + if err != nil || retitled.Outcome != WriteOutcomeSkipped { + t.Fatalf("same-title write = (%+v, %v), want a skip", retitled, err) + } + + // The other owner is NOT deduplicated against: beta may hold a genuinely + // different truth about tabs. + beta, err := graph.Write(WriteRequest{Owner: OwnerProject("beta"), Type: MemoryPreference, + Title: "Prefers tabs", Text: "Prefers tabs over spaces in Go files."}) + if err != nil || beta.Outcome != WriteOutcomeAdded { + t.Fatalf("write into beta = (%+v, %v), want an add — the owners are different", beta, err) + } + + // Both outcomes are journaled. + skips, err := graph.db.Query(`SELECT COUNT(*) FROM events WHERE kind = ?`, EventMemorySkipped) + if err != nil { + t.Fatal(err) + } + defer skips.Close() + var count int + for skips.Next() { + if err := skips.Scan(&count); err != nil { + t.Fatal(err) + } + } + if count != 2 { + t.Fatalf("skips journaled = %d, want 2", count) + } +} + +// The dedup fallback holds on a store whose FTS index never existed: the same +// door, the same rule, a bounded scan instead of the index. +func TestWriteDoorDeduplicatesWithoutTheIndex(t *testing.T) { + path := filepath.Join(t.TempDir(), "noindex.db") + graph := openTestStore(t, path) + // Retire the index and tell the handle, the way a probe-failed open would + // have: the degraded shape is store.fts = false with no index table. + if _, err := graph.db.Exec(`DROP TABLE memories_fts`); err != nil { + t.Fatal(err) + } + graph.fts = false + + first, err := graph.Write(WriteRequest{Owner: OwnerUser, Type: MemoryFact, + Title: "Amber key", Text: "The amber key opens the archive."}) + if err != nil || first.Outcome != WriteOutcomeAdded { + t.Fatalf("first write without an index = (%+v, %v)", first, err) + } + again, err := graph.Write(WriteRequest{Owner: OwnerUser, Type: MemoryFact, + Title: "Amber key", Text: "The AMBER key opens the archive."}) + if err != nil || again.Outcome != WriteOutcomeSkipped { + t.Fatalf("repeat without an index = (%+v, %v), want a skip", again, err) + } + // And the degraded search answers nil — a miss, not an error. + hits, err := graph.SearchMemories([]string{OwnerUser}, "amber", 5) + if err != nil || hits != nil { + t.Fatalf("search without an index = (%v, %v), want (nil, nil)", hits, err) + } + // The no-index candidates pool still ranks, on the arithmetic tiers only. + pool, err := graph.MemoryCandidates([]string{OwnerUser}, "amber", 8) + if err != nil || len(pool) != 1 { + t.Fatalf("pool without an index = (%+v, %v), want the row on recency alone", pool, err) + } +} + +// ── the failure journal ───────────────────────────────────────────────────── + +// A write failure that reaches the journal is readable, with the door that +// failed and the reason. Errors at these doors used to vanish. +func TestJournalMemoryFailureIsReadable(t *testing.T) { + graph := openTestStore(t, filepath.Join(t.TempDir(), "failures.db")) + if err := graph.JournalMemoryFailure("extract", errors.New("the decider answered nonsense")); err != nil { + t.Fatalf("JournalMemoryFailure: %v", err) + } + if err := graph.JournalMemoryFailure("import", errors.New("row 3 refused")); err != nil { + t.Fatalf("JournalMemoryFailure: %v", err) + } + events, err := graph.Events(0, 100) + if err != nil { + t.Fatal(err) + } + var failures []map[string]string + for _, event := range events { + if event.Kind != EventMemoryWriteFailed { + continue + } + var payload map[string]string + if err := json.Unmarshal(event.Payload, &payload); err != nil { + t.Fatalf("decode %d: %v", event.Seq, err) + } + failures = append(failures, payload) + } + if len(failures) != 2 { + t.Fatalf("failure events = %d, want 2", len(failures)) + } + if failures[0]["via"] != "extract" || !strings.Contains(failures[0]["error"], "decider") { + t.Fatalf("first failure = %+v, want via=extract and the reason", failures[0]) + } + // A nil error is nothing to journal — the callers pass what they have. + if err := graph.JournalMemoryFailure("extract", nil); err != nil { + t.Fatalf("JournalMemoryFailure(nil) = %v, want a quiet no", err) + } +} + +// ── quarantine ────────────────────────────────────────────────────────────── + +// A bare project scope — the legacy caller's spelling — lands in quarantine: +// visible to the person's audit reads, invisible to every injection. +func TestABareProjectScopeQuarantinesAndTheQuarantineNeverInjects(t *testing.T) { + graph := openTestStore(t, filepath.Join(t.TempDir(), "quarantine.db")) + quarantined := mustAddMemory(t, graph, Memory{Type: MemoryDecision, Scope: MemoryScopeProject, + Title: "Pricing stays annual", Text: "Pricing is billed annually, decided in the alpha project."}) + if quarantined.Owner != OwnerLegacyProject { + t.Fatalf("a bare project write landed as %q, want %q", quarantined.Owner, OwnerLegacyProject) + } + if !containsStubTag(quarantined.Tags, OwnerLegacyTag) { + t.Fatalf("quarantined row tags = %v, want the legacy marker", quarantined.Tags) + } + // A REAL view — the owners a session names — never contains the quarantine. + realView := []string{OwnerUser, OwnerMachine, OwnerProject("alpha")} + pool, err := graph.MemoryCandidates(realView, "pricing annual", MemoryCandidatesDefault) + if err != nil { + t.Fatal(err) + } + if containsMemoryStub(pool, quarantined.ID) { + t.Errorf("a quarantined row reached the pool: %+v", pool) + } + hits, err := graph.SearchMemories([]string{OwnerUser}, "pricing", 5) + if err != nil || len(hits) != 0 { + t.Fatalf("a user search saw quarantine: (%v, %v)", memoryIDs(hits), err) + } + listed, err := graph.QuarantinedMemories(50) + if err != nil || len(listed) != 1 || listed[0].ID != quarantined.ID { + t.Fatalf("QuarantinedMemories = (%v, %v), want the row", memoryIDs(listed), err) + } +} + +// Re-homing moves a quarantined row to a provable owner, journaled, and can +// never touch a row that already has an owner. +func TestRehomeMovesOnlyQuarantinedRowsAndJournalsTheMove(t *testing.T) { + graph := openTestStore(t, filepath.Join(t.TempDir(), "rehome.db")) + quarantined := mustAddMemory(t, graph, Memory{Type: MemoryDecision, Scope: MemoryScopeProject, + Title: "Pricing stays annual", Text: "Pricing is billed annually."}) + held := mustAddMemory(t, graph, Memory{Owner: OwnerUser, Type: MemoryPreference, + Title: "Wants terse replies", Text: "Answers short, conclusion first."}) + + alpha := OwnerProject("alpha") + moved, err := graph.RehomeMemories([]string{quarantined.ID, held.ID, "mem_absent"}, alpha) + if err != nil || moved != 1 { + t.Fatalf("RehomeMemories = (%d, %v), want exactly the one quarantined row", moved, err) + } + got, err := graph.GetMemories([]string{alpha}, []string{quarantined.ID}) + if err != nil || len(got) != 1 { + t.Fatalf("the re-homed row under alpha = (%v, %v)", memoryIDs(got), err) + } + if !containsStubTag(got[0].Tags, OwnerLegacyTag) { + t.Errorf("the re-homed row lost its marker: %v", got[0].Tags) + } + // The row with an owner was untouched. + unchanged, err := graph.GetMemories([]string{OwnerUser}, []string{held.ID}) + if err != nil || len(unchanged) != 1 || unchanged[0].Owner != OwnerUser { + t.Fatalf("a re-home touched an owned row: (%+v, %v)", unchanged, err) + } + // A re-home to an owner nobody mints is refused. + if _, err := graph.RehomeMemories([]string{quarantined.ID}, "team:the-fold"); !errors.Is(err, ErrInvalid) { + t.Errorf("RehomeMemories(team) = %v, want a refusal", err) + } + // The move is journaled. + events, err := graph.Events(0, 100) + if err != nil { + t.Fatal(err) + } + rehomed := 0 + for _, event := range events { + if event.Kind == EventMemoryRehomed { + rehomed++ + } + } + if rehomed != 1 { + t.Fatalf("rehome events = %d, want 1", rehomed) + } +} + +// ── the migration ─────────────────────────────────────────────────────────── + +// A store written before owners existed comes out of the migration with every +// row owned the way its scope proves, and the unknown-project rows in +// quarantine — never re-attributed to the person at large. +func TestTheMigrationOwnsEveryRowWithoutGuessing(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "legacy.db") + graph := openTestStore(t, path) + legacy := map[string]string{ + "user": "A preference the person stated.", + "env": "A fact about this machine.", + "project": "A decision from a project nobody can name.", + } + for scope, text := range legacy { + mustAddMemory(t, graph, Memory{Type: MemoryFact, Scope: scope, Title: "Legacy row", Text: text}) + } + if err := graph.Close(); err != nil { + t.Fatal(err) + } + // Reopen: the migration runs on the open. + graph, err := Open(path) + if err != nil { + t.Fatalf("reopen: %v", err) + } + defer graph.Close() + rows, err := graph.queryMemories(``, nil, `id`, 0) + if err != nil { + t.Fatal(err) + } + owners := map[string]string{} + for _, row := range rows { + owners[row.Text] = row.Owner + } + if got := owners[legacy["user"]]; got != OwnerUser { + t.Errorf("the user row owns as %q, want %q", got, OwnerUser) + } + if got := owners[legacy["env"]]; got != OwnerMachine { + t.Errorf("the env row owns as %q, want %q", got, OwnerMachine) + } + if got := owners[legacy["project"]]; got != OwnerLegacyProject { + t.Errorf("the project row owns as %q, want %q — a guess is how a row widens", got, OwnerLegacyProject) + } + // AND THE MIGRATION IS IDEMPOTENT: a second open changes nothing. + if err := graph.Close(); err != nil { + t.Fatal(err) + } + graph, err = Open(path) + if err != nil { + t.Fatal(err) + } + defer graph.Close() + rows2, err := graph.queryMemories(``, nil, `id`, 0) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(rows, rows2) { + t.Fatalf("a second migration changed rows:\nfirst %+v\nsecond %+v", rows, rows2) + } +} + +// ── pagination ────────────────────────────────────────────────────────────── + +// Pages walk the same ordering ListMemories uses, with a cursor that survives +// writes landing between pages, and end with More=false. +func TestMemoryPageWalksAStableCursor(t *testing.T) { + graph := openTestStore(t, filepath.Join(t.TempDir(), "pages.db")) + const total = 25 + ids := make([]string, 0, total) + for index := 0; index < total; index++ { + memory := mustAddMemory(t, graph, Memory{Owner: OwnerUser, Type: MemoryFact, + Title: fmt.Sprintf("Page row %02d", index), + Text: fmt.Sprintf("Page row number %02d, distinct words %d.", index, index*7)}) + ids = append(ids, memory.ID) + } + var seen []string + cursor := MemoryCursor{} + pages := 0 + for { + page, next, err := graph.MemoryPage([]string{OwnerUser}, cursor, 10) + if err != nil { + t.Fatalf("page %d: %v", pages, err) + } + pages++ + seen = append(seen, memoryIDs(page)...) + if !next.More { + break + } + cursor = next + if pages > 10 { + t.Fatal("the cursor never ended") + } + } + if pages != 3 { + t.Fatalf("pages = %d, want 3", pages) + } + // NEWEST TOUCHED FIRST is the list order, so the walk reads the ids the + // test minted oldest-first backwards. + want := make([]string, len(ids)) + for index, id := range ids { + want[len(ids)-1-index] = id + } + if !reflect.DeepEqual(seen, want) { + t.Fatalf("walked %v, want newest-touched order %v", seen, want) + } + // A write landing between pages does not move the walk: the cursor is the + // position, not an offset. + first, next, err := graph.MemoryPage([]string{OwnerUser}, MemoryCursor{}, 10) + if err != nil { + t.Fatal(err) + } + mustAddMemory(t, graph, Memory{Owner: OwnerUser, Type: MemoryFact, Title: "Landed between", Text: "A fresh row between two pages."}) + second, _, err := graph.MemoryPage([]string{OwnerUser}, next, 10) + if err != nil { + t.Fatal(err) + } + for _, row := range append(append([]Memory{}, first...), second...) { + if row.Title == "Landed between" { + t.Fatalf("the interloper appeared on pages already walked: %+v", row) + } + } + // Owners hold: another project's rows are on no page of this walk. + other, err := graph.Write(WriteRequest{Owner: OwnerProject("elsewhere"), Type: MemoryFact, Title: "Elsewhere", Text: "Another project's truth."}) + if err != nil { + t.Fatal(err) + } + page, _, err := graph.MemoryPage([]string{OwnerUser}, MemoryCursor{}, 100) + if err != nil { + t.Fatal(err) + } + if containsMemoryStub(toStubs(page), other.Memory.ID) { + t.Errorf("another owner's row is on this page: %+v", page) + } +} + +func toStubs(memories []Memory) []MemoryStub { + stubs := make([]MemoryStub, 0, len(memories)) + for _, memory := range memories { + stubs = append(stubs, MemoryStub{ID: memory.ID, Title: memory.Title, Type: memory.Type, Scope: memory.Scope}) + } + return stubs +} + +func containsStubTag(tags []string, want string) bool { + for _, tag := range tags { + if tag == want { + return true + } + } + return false +} + +// ── regression: owner widening ─────────────────────────────────────────────── + +// An unknown owner in replay — a future team:… owner from a newer build — is +// quarantined, never widened to user. A row whose owner cannot be understood +// must never degrade to one visible from every project. +func TestOwnerForReplayQuarantinesAnUnknownOwner(t *testing.T) { + // A payload with an explicit future team owner. + payload := memoryPayload{ID: "mem_x", Owner: "team:the-fold", Scope: MemoryScopeUser} + got := ownerForReplay(payload) + if got != OwnerLegacyProject { + t.Fatalf("ownerForReplay(team:the-fold) = %q, want %q (quarantine)", got, OwnerLegacyProject) + } + // An unknown scope word with no explicit owner also quarantines. + payload2 := memoryPayload{ID: "mem_y", Scope: "global"} + got2 := ownerForReplay(payload2) + if got2 != OwnerLegacyProject { + t.Fatalf("ownerForReplay(scope=global) = %q, want %q (quarantine)", got2, OwnerLegacyProject) + } +} + +// A memory event with an unknown owner — a newer build's team:… owner — is +// quarantined by the sync fold rather than widened. +func TestMemoryEventOwnerQuarantinesAnUnknownOwner(t *testing.T) { + // A payload carrying an explicit owner this build does not mint. + payload, _ := json.Marshal(memoryPayload{ID: "mem_x", Owner: "team:the-fold", Scope: MemoryScopeUser}) + event := Event{Kind: EventMemoryAdd, Payload: json.RawMessage(payload)} + got := memoryEventOwner(event) + if got != OwnerLegacyProject { + t.Fatalf("memoryEventOwner(team:the-fold) = %q, want %q (quarantine)", got, OwnerLegacyProject) + } + // An unparseable payload also quarantines — a blob this build cannot decode + // carries no owner it can assert. + event2 := Event{Kind: EventMemoryAdd, Payload: json.RawMessage(`{bogus`)} + got2 := memoryEventOwner(event2) + if got2 != OwnerLegacyProject { + t.Fatalf("memoryEventOwner(unparseable) = %q, want %q (quarantine)", got2, OwnerLegacyProject) + } +} + +// ── regression: owner-scoped write doors ───────────────────────────────────── + +// A SupersedeMemoryForOwners that names a row the owners cannot see is refused. +func TestSupersedeForOwnersRefusesAForeignID(t *testing.T) { + graph := openTestStore(t, filepath.Join(t.TempDir(), "supersede-refuse.db")) + alpha := OwnerProject("alpha") + beta := OwnerProject("beta") + + alphaRow, err := graph.Write(WriteRequest{Owner: alpha, Type: MemoryFact, + Title: "Deploys Tuesdays", Text: "Deploys to the amber cluster every Tuesday."}) + if err != nil { + t.Fatal(err) + } + // Beta tries to supersede alpha's row through the scoped door. + _, err = graph.SupersedeMemoryForOwners([]string{beta}, alphaRow.Memory.ID, Memory{ + Owner: beta, Type: MemoryFact, + Title: "Deploys Wednesdays", Text: "Deploys to the ember cluster every Wednesday.", + }) + if err == nil { + t.Fatal("SupersedeMemoryForOwners accepted a foreign id — the door is open") + } + if !errors.Is(err, ErrInvalid) { + t.Fatalf("SupersedeMemoryForOwners = %v, want an ErrInvalid refusal", err) + } +} + +// An UpdateMemoryForOwners that names a row the owners cannot see is refused. +func TestUpdateForOwnersRefusesAForeignID(t *testing.T) { + graph := openTestStore(t, filepath.Join(t.TempDir(), "update-refuse.db")) + alpha := OwnerProject("alpha") + beta := OwnerProject("beta") + + alphaRow, err := graph.Write(WriteRequest{Owner: alpha, Type: MemoryFact, + Title: "Amber cluster", Text: "The amber cluster is in us-east-1."}) + if err != nil { + t.Fatal(err) + } + // Beta tries to update alpha's row through the scoped door. + err = graph.UpdateMemoryForOwners([]string{beta}, alphaRow.Memory.ID, + "Amber cluster", "The amber cluster is in us-west-2.", nil, "") + if err == nil { + t.Fatal("UpdateMemoryForOwners accepted a foreign id — the door is open") + } + if !errors.Is(err, ErrInvalid) { + t.Fatalf("UpdateMemoryForOwners = %v, want an ErrInvalid refusal", err) + } +} + +// ── regression: rehome fold validates the target owner ─────────────────────── + +// A rehomed event in the fold whose target owner is not valid is skipped — +// never applied to move a quarantined row to an owner this build does not mint. +func TestRehomedFoldRefusesAnInvalidTargetOwner(t *testing.T) { + graph := openTestStore(t, filepath.Join(t.TempDir(), "rehome-fold-refuse.db")) + // Put a row in quarantine. + quarantined := mustAddMemory(t, graph, Memory{Type: MemoryDecision, Scope: MemoryScopeProject, + Title: "Pricing stays annual", Text: "Pricing is billed annually."}) + if quarantined.Owner != OwnerLegacyProject { + t.Fatalf("quarantined row owner = %q, want %q", quarantined.Owner, OwnerLegacyProject) + } + // Construct a rehome event with a future team owner. + payload, _ := json.Marshal(map[string]any{ + "ids": []string{quarantined.ID}, + "owner": "team:the-fold", + }) + events := []MemoryEvent{{Seq: 1, Kind: EventMemoryRehomed, Payload: json.RawMessage(payload)}} + // Apply with a permissive policy — the fold itself should refuse the invalid + // owner inside the apply. + result, err := graph.ApplyMemoryEvents(events, nil) + if err != nil { + t.Fatal(err) + } + if result.Applied != 0 { + t.Fatalf("the rehome fold applied a future team owner: %d applied", result.Applied) + } + if result.Skipped == 0 { + t.Fatal("the rehome fold did not skip the invalid-owner event") + } + // The row is still in quarantine. + row, ok, err := graph.MemoryRecord(quarantined.ID) + if err != nil || !ok || row.Owner != OwnerLegacyProject { + t.Fatalf("after invalid-owner rehome fold: (%+v, %v, %v), want still in quarantine", row, ok, err) + } +} + +// ── regression: aged dedup ─────────────────────────────────────────────────── + +// The dedup door catches an exact duplicate that is older than the old 5-row +// FTS window. The store's Write door is the one path; prove it skips an exact +// repeat that is behind more than five active rows for the same owner. +func TestDedupCatchesAnExactDuplicateBeyondTheOldWindow(t *testing.T) { + graph := openTestStore(t, filepath.Join(t.TempDir(), "aged-dedup.db")) + owner := OwnerUser + // Write six fillers, then a target, then the duplicate — six rows after the + // target means the old 5-neighbor window would miss it. + for index := 0; index < 6; index++ { + if _, err := graph.Write(WriteRequest{Owner: owner, Type: MemoryFact, + Title: fmt.Sprintf("Filler %02d", index), + Text: fmt.Sprintf("Filler row number %02d with distinct words %x.", index, index)}); err != nil { + t.Fatal(err) + } + } + first, err := graph.Write(WriteRequest{Owner: owner, Type: MemoryPreference, + Title: "Prefers tabs", Text: "Prefers tabs over spaces in Go files."}) + if err != nil || first.Outcome != WriteOutcomeAdded { + t.Fatalf("first write of the target: (%+v, %v)", first, err) + } + // Six more fillers push the target past the old 5-neighbor window. + for index := 6; index < 12; index++ { + if _, err := graph.Write(WriteRequest{Owner: owner, Type: MemoryFact, + Title: fmt.Sprintf("Filler %02d", index), + Text: fmt.Sprintf("Filler row number %02d with distinct words %x.", index, index)}); err != nil { + t.Fatal(err) + } + } + // The exact repeat, different case and spacing. + again, err := graph.Write(WriteRequest{Owner: owner, Type: MemoryPreference, + Title: "Prefers tabs", Text: "PREFERS tabs over spaces in Go files."}) + if err != nil || again.Outcome != WriteOutcomeSkipped { + t.Fatalf("repeat past the old window = (%+v, %v), want a skip", again, err) + } + if again.Memory.ID != first.Memory.ID { + t.Fatalf("the aged dedup matched %q, want the original %q", again.Memory.ID, first.Memory.ID) + } +} diff --git a/internal/store/memory_sync.go b/internal/store/memory_sync.go new file mode 100644 index 0000000000..cf37e0da94 --- /dev/null +++ b/internal/store/memory_sync.go @@ -0,0 +1,442 @@ +package store + +import ( + "database/sql" + "encoding/json" + "errors" + "fmt" +) + +// The memory sync seam. THESE TWO METHODS ARE THE WHOLE OF WHAT SYNC NEEDS +// from the store, and neither of them knows what a transport is: one reads the +// journal's memory events after a cursor, the other folds events that arrived. +// PR #1738's sealed-object transport will be the thing that moves these +// batches; until a transport exists NOTHING calls them except tests — which is +// exactly what "ready" means here. +// +// The journal is already the replication log this needs: +// +// - events are append-only and immutable, so export is a scan, not a merge; +// - every payload carries the owner, so the applying side can enforce its +// own visibility policy before it lands anything; +// - memory rows are only ever written by these same event kinds, so an +// applied event and a local write are indistinguishable to every reader — +// including Rebuild, which replays the journal and reproduces the fold. +// +// WHAT THIS SEAM DOES NOT DECIDE: which owners may arrive (the caller's policy +// answers that), who pairs with whom, how events travel, or whether a remote +// device is who it says it is. Transport and trust are Phase 4's; the fold is +// here because the fold is the only part that must be identical on every +// device. +// +// ANALYTICS NEVER MASQUERADE AS REPLICATION. Ranking snapshots +// ([EventMemoryRanking]) and the outcome counters are this store's own +// evidence about its own model calls; they travel nowhere, because another +// device's evidence about ITS calls is not a fact about this one. + +// MemoryEvent is one journal entry on the wire: the Event a store carries, +// with nothing local added. The sequence is the EXPORTING store's own — a +// receiver keeps cursors per origin and never re-uses a foreign seq as its own. +type MemoryEvent struct { + Seq int64 + Time string + NodeID string + Kind EventKind + Payload json.RawMessage +} + +// memorySyncKinds are the kinds a memory sync moves. Every one of them is a +// kind whose payload the replay already understands, which is what makes an +// applied batch and a replayed journal the same fold. +func memorySyncKinds() []EventKind { + return []EventKind{ + EventMemoryAdd, EventMemoryUpdate, EventMemorySupersede, + EventMemoryForget, EventMemoryRestore, + EventMemorySkipped, EventMemoryWriteFailed, EventMemoryRehomed, + } +} + +// ExportMemoryEvents reads the journal's memory events after a cursor, oldest +// first, bounded. The returned cursor is the last event's seq: hand it back on +// the next call and the export continues exactly where it stopped. A batch +// smaller than the limit has more=false — the export is drained. +func (s *Store) ExportMemoryEvents(after int64, limit int) ([]MemoryEvent, int64, bool, error) { + if limit <= 0 { + limit = 500 + } + kinds := memorySyncKinds() + placeholders := "" + args := make([]any, 0, len(kinds)+2) + args = append(args, after) + for _, kind := range kinds { + if placeholders != "" { + placeholders += ", " + } + placeholders += "?" + args = append(args, string(kind)) + } + args = append(args, limit+1) + rows, err := s.db.Query(` + SELECT seq, ts, node_id, kind, payload + FROM events + WHERE seq > ? AND kind IN (`+placeholders+`) + ORDER BY seq LIMIT ?`, args...) + if err != nil { + return nil, 0, false, fmt.Errorf("export memory events: %w", err) + } + defer rows.Close() + events := make([]MemoryEvent, 0, limit) + last := after + for rows.Next() { + var event MemoryEvent + var timestamp, payload string + if err := rows.Scan(&event.Seq, ×tamp, &event.NodeID, &event.Kind, &payload); err != nil { + return nil, 0, false, fmt.Errorf("export memory events: %w", err) + } + event.Time = timestamp + event.Payload = json.RawMessage(payload) + events = append(events, event) + last = event.Seq + } + if err := rows.Err(); err != nil { + return nil, 0, false, fmt.Errorf("export memory events: %w", err) + } + more := len(events) > limit + if more { + events = events[:limit] + last = events[len(events)-1].Seq + } + return events, last, more, nil +} + +// AppliedMemoryEvents is what ApplyMemoryEvents did with a batch. +type AppliedMemoryEvents struct { + // Applied counts events that changed this store. + Applied int + // Skipped counts events that arrived as no-ops: an id already here, a + // tombstone that already stands, a supersession whose target this store + // retired first, a change outside the receiver's owner policy. Skipped is + // SUCCESS — it is what makes re-delivery safe and what makes the fold + // deterministic instead of conflict-free-by-fiat. + Skipped int +} + +// ApplyMemoryEvents folds a batch from another store into this one, in the +// order the batch carries. +// +// THE FOLD RULES ARE DETERMINISTIC AND STRUCTURAL, and they are exactly the +// replay rules the local journal already follows: +// +// - add of an id this store already holds is a skip (idempotent re-delivery); +// - update and supersede apply only against an ACTIVE row, and only within +// the owner the event or the row itself names — a store that has retired +// the target first is not overwritten back to life by a slower delivery; +// - forget is a tombstone and applies to any row that is not already +// forgotten; a LATER-delivered add of the same id never resurrects it, +// because the add checks existence, not activity; +// - restore brings back only a forgotten row; +// - re-home moves only a row that is still quarantined, exactly what the +// live door would have moved. +// +// Two devices superseding the same row, or one forgetting what the other +// updates, therefore converge to the first fold that landed, and the skipped +// half is counted so a surface can say so. That is an ORDER, not a promise +// that disagreement cannot happen. +// +// Each applied event is RE-JOURNALED under the receiving store's own sequence +// with its original payload, so the fold is durable, replayable, and +// indistinguishable in shape from local history. A skipped event is journaled +// NOWHERE — a no-op is not history, and the transaction that journaled it +// rolls back with the fold. +// +// The allow function is the RECEIVER's visibility policy, decided before +// anything lands: an owner the policy refuses is skipped, never written under +// a widened owner. Update, forget and restore payloads carry no owner of their +// own — they act on a row that is already here — so for those the policy is +// asked about THE ROW'S OWN owner, read in the same transaction. A nil policy +// allows every owner, which is the local default — a store moving its own +// journal forward in a test. +func (s *Store) ApplyMemoryEvents(events []MemoryEvent, allow func(owner string) bool) (AppliedMemoryEvents, error) { + var result AppliedMemoryEvents + for _, event := range events { + applied, err := s.applyOneMemoryEvent(event, allow) + if err != nil { + return result, fmt.Errorf("apply memory event %d (%s): %w", event.Seq, event.Kind, err) + } + if applied { + result.Applied++ + } else { + result.Skipped++ + } + } + return result, nil +} + +func (s *Store) applyOneMemoryEvent(event MemoryEvent, allow func(owner string) bool) (bool, error) { + if !eventSyncable(event.Kind) { + // A LATER BUILD'S KINDS ARE NOT SILENTLY DROPPED. They arrive here as + // an error, so an old build receiving a new journal reports it instead + // of losing a change nobody will ever account for. + return false, fmt.Errorf("%w: %q is not a memory event", ErrInvalid, event.Kind) + } + // THE OWNER IS READ BEFORE ANYTHING ELSE, because the policy is what makes + // a receiver more than a dump. An event whose payload cannot say its owner + // is an event from before the column existed; it is owned the way the + // replay owns such a row, from its scope. The kinds that act on a row that + // is already here (update, forget, restore) are checked INSIDE the fold, + // against the row's own owner — a payload that names no owner cannot be + // gated here without inventing one. + if ownerBearingKind(event.Kind) { + if allow != nil && !allow(memoryEventOwner(Event{Kind: event.Kind, Payload: event.Payload})) { + return false, nil + } + } + tx, err := s.beginWrite() + if err != nil { + return false, err + } + defer tx.Rollback() + // The event is journaled under THIS store's sequence first; the fold below + // stamps its views with that sequence. A skip rolls the journal entry back + // with the rest of the transaction — a no-op is not history. + seq, _, err := appendEvent(tx, "remote", event.Kind, event.Payload) + if err != nil { + return false, err + } + local := Event{Seq: seq, NodeID: "remote", Kind: event.Kind, Payload: event.Payload} + applied, err := foldMemoryEvent(tx, local, seq, s.fts, allow) + if err != nil { + return false, err + } + if !applied { + return false, nil + } + if err := tx.Commit(); err != nil { + return false, err + } + return true, nil +} + +// ownerBearingKind answers whether an event's payload itself names the owner +// the policy is asked about. Add, supersede and re-home create or re-own +// rows, so their payloads carry the owner; update, forget and restore act on +// rows that already exist here and carry none. +func ownerBearingKind(kind EventKind) bool { + switch kind { + case EventMemoryAdd, EventMemorySupersede, EventMemoryRehomed: + return true + } + return false +} + +// foldMemoryEvent applies ONE memory event to the views, idempotently, and +// answers whether it changed anything. IT IS THE SHARED FOLD: the write +// path's replay reaches it through [replayEvent] with the journal's own seq, +// and the sync seam reaches it with the re-journaled event's seq. One fold, +// so no receiver can disagree with its own replay about what an event means. +func foldMemoryEvent(tx *sql.Tx, event Event, seq int64, fts bool, allow func(owner string) bool) (bool, error) { + switch event.Kind { + case EventMemoryAdd: + var payload memoryPayload + if err := json.Unmarshal(event.Payload, &payload); err != nil { + return false, err + } + payload.Owner = ownerForReplay(payload) + var exists int + if err := tx.QueryRow(`SELECT COUNT(*) FROM memories WHERE id = ?`, payload.ID).Scan(&exists); err != nil { + return false, err + } + if exists != 0 { + // ALREADY HERE. An add that re-arrives is re-delivery, and + // re-delivery is a skip — never a second row, never a resurrection + // of something this store has since retired. + return false, nil + } + return true, applyMemoryAdd(tx, payload, seq, fts) + case EventMemoryUpdate: + var payload memoryUpdatePayload + if err := json.Unmarshal(event.Payload, &payload); err != nil { + return false, err + } + // THE GUARD'S OWNER IS THE ROW'S OWN. An update event carries no owner + // — it acts on a row that is already here, under whatever owner it + // arrived with — so the row itself is read and the receiver's policy + // is asked about THAT. A user-labeled context must never be allowed to + // rewrite a project's row: an event that cannot say whose row it + // touches is gated by the row, not by a default. + rowOwner, active := memoryRowOwner(tx, payload.ID) + if !active { + return false, nil + } + if allow != nil && !allow(rowOwner) { + return false, nil + } + return true, applyMemoryUpdate(tx, payload, seq, fts) + case EventMemorySupersede: + var payload memorySupersedePayload + if err := json.Unmarshal(event.Payload, &payload); err != nil { + return false, err + } + payload.New.Owner = ownerForReplay(payload.New) + // THE GUARD'S OWNER IS THE REPLACEMENT'S — a supersede is one decision + // about the replacement's owner, and the wrapper payload does not carry + // the owner at the top level for the policy read above to find. + if !memoryActiveForOwner(tx, payload.OldID, payload.New.Owner) { + // THE TARGET IS ALREADY GONE HERE — retired or forgotten by this + // store's own history or by a faster delivery. Applying would + // resurrect a retired row or add an orphan; skipping is the fold + // rule, and the chain stays consistent with it. + return false, nil + } + var exists int + if err := tx.QueryRow(`SELECT COUNT(*) FROM memories WHERE id = ?`, payload.New.ID).Scan(&exists); err != nil { + return false, err + } + if exists != 0 { + return false, nil + } + return true, applyMemorySupersede(tx, payload, seq, fts) + case EventMemoryForget: + var payload memoryForgetPayload + if err := json.Unmarshal(event.Payload, &payload); err != nil { + return false, err + } + var status, rowOwner string + err := tx.QueryRow(`SELECT owner, status FROM memories WHERE id = ?`, payload.ID).Scan(&rowOwner, &status) + switch { + case errors.Is(err, sql.ErrNoRows): + // NOTHING TO FORGET HERE. The tombstone may arrive before the row + // it names on another device, but on this one it is a skip. + return false, nil + case err != nil: + return false, err + case status == MemoryForgotten: + return false, nil + } + // THE POLICY IS ASKED ABOUT THE ROW'S OWN OWNER — the payload names no + // owner, and a tombstone the policy would refuse must not land because + // the payload stayed silent. + if allow != nil && !allow(rowOwner) { + return false, nil + } + return true, applyMemoryForget(tx, payload, seq, fts) + case EventMemoryRestore: + var payload memoryRestorePayload + if err := json.Unmarshal(event.Payload, &payload); err != nil { + return false, err + } + var status, rowOwner string + err := tx.QueryRow(`SELECT owner, status FROM memories WHERE id = ?`, payload.ID).Scan(&rowOwner, &status) + switch { + case errors.Is(err, sql.ErrNoRows): + return false, nil + case err != nil: + return false, err + case status != MemoryForgotten: + return false, nil + } + if allow != nil && !allow(rowOwner) { + return false, nil + } + return true, applyMemoryRestore(tx, payload, seq, fts) + case EventMemoryRehomed: + // A RE-HOME FROM THE ORIGIN APPLIES, policy-gated at the top of the + // fold: the origin proved the row's project, and a receiver that + // refused the news would keep a row in quarantine the origin has since + // re-homed — divergence with no upside. The fold under it only ever + // moves a row that is STILL quarantined ([applyRehomeOne]), so a + // re-home can never strip an owner it disagrees with. + // + // THE TARGET OWNER IS VALIDATED, same as the live door. A re-home + // event carrying an owner this build does not mint is refused: moving + // a quarantined row to an invalid owner would be a quarantine escape. + var payload struct { + IDs []string `json:"ids"` + } + if err := json.Unmarshal(event.Payload, &payload); err != nil { + return false, err + } + targetOwner := memoryEventOwner(event) + if !ValidOwner(targetOwner) { + // An owner this build does not understand is refused, not widened. + return false, nil + } + // A RE-HOME TO QUARANTINE IS NOT A RE-HOME. The live door never moves a + // row from quarantine to quarantine; the fold must agree. A row already + // in quarantine that an event tries to "re-home" to OwnerLegacyProject + // stays exactly where it is. + if targetOwner == OwnerLegacyProject { + return false, nil + } + moved := 0 + for _, id := range payload.IDs { + applied, err := applyRehomeOne(tx, id, targetOwner, seq) + if err != nil { + return false, err + } + if applied { + moved++ + } + } + return moved > 0, nil + default: + // Skipped and write-failed are observability events about the SENDING + // device's own attempts: they say what that device went through, and + // applying them here would manufacture local views out of somebody + // else's attempt — and a no-op is not history, so the journal entry + // the fold's caller wrote rolls back with the skip. + return false, nil + } +} + +// memoryRowOwner answers the owner of an ACTIVE row, and whether it is active +// at all. It is the guard an in-place fold walks before it writes: an update +// that cannot find an active row changes nothing, and the owner it answers is +// what the receiver's policy is asked about. +func memoryRowOwner(tx *sql.Tx, id string) (string, bool) { + var owner, status string + if err := tx.QueryRow(`SELECT owner, status FROM memories WHERE id = ?`, id).Scan(&owner, &status); err != nil { + return "", false + } + return owner, status == MemoryActive +} + +// memoryActiveForOwner answers whether an id is an active row under the +// event's own owner — the read a fold must do before an in-place change, so no +// receiver can move another owner's row. Applied from a transaction, it takes +// the querier rather than the store. +func memoryActiveForOwner(tx *sql.Tx, id, owner string) bool { + var count int + if err := tx.QueryRow(`SELECT COUNT(*) FROM memories WHERE id = ? AND owner = ? AND status = ?`, + id, owner, MemoryActive).Scan(&count); err != nil { + return false + } + return count == 1 +} + +// eventSyncable answers whether an event kind is one the fold understands at +// all. Kinds this build does not know are refused loudly by the apply door — +// see [Store.ApplyMemoryEvents] — while the replay treats an unknown kind as +// the error it has always been. +func eventSyncable(kind EventKind) bool { + for _, candidate := range memorySyncKinds() { + if candidate == kind { + return true + } + } + return false +} + +// memoryEventOwner reads the owner out of whichever payload shape the event +// carries, for the policy's sake. An event from before the column answers the +// way the replay answers it. An owner this build does not understand is +// quarantined — a policy gate that widens to user is not a gate. +func memoryEventOwner(event Event) string { + var payload memoryPayload + if err := json.Unmarshal(event.Payload, &payload); err == nil { + return ownerForReplay(payload) + } + // A payload this build cannot parse carries no owner. The quarantine is the + // honest answer for a row whose provenance is unknown. + return OwnerLegacyProject +} diff --git a/internal/store/memory_sync_test.go b/internal/store/memory_sync_test.go new file mode 100644 index 0000000000..aab17359b1 --- /dev/null +++ b/internal/store/memory_sync_test.go @@ -0,0 +1,287 @@ +package store + +import ( + "encoding/json" + "path/filepath" + "testing" +) + +// The sync seam's fold, proven between two stores. Nothing here knows what a +// transport is: the batch is taken from one journal and handed to the other, +// which is what any transport will do. +func TestMemorySyncFoldsAcrossTwoStoresIdempotently(t *testing.T) { + origin := openTestStore(t, filepath.Join(t.TempDir(), "origin.db")) + receiver := openTestStore(t, filepath.Join(t.TempDir(), "receiver.db")) + + // The origin writes under owners its policy allows, and one the receiver + // must refuse. + userRow, err := origin.Write(WriteRequest{Owner: OwnerUser, Type: MemoryPreference, + Title: "Wants terse replies", Text: "Answers short, conclusion first."}) + if err != nil { + t.Fatal(err) + } + machineRow, err := origin.Write(WriteRequest{Owner: OwnerMachine, Type: MemoryFact, + Title: "Runs on spark", Text: "The heavy jobs run on the spark machine."}) + if err != nil { + t.Fatal(err) + } + if _, err := origin.Write(WriteRequest{Owner: OwnerProject("secret"), Type: MemoryDecision, + Title: "Secret pricing", Text: "The secret project's pricing is X."}); err != nil { + t.Fatal(err) + } + // A supersession, so the chain crosses. + fresh, err := origin.SupersedeMemory(machineRow.Memory.ID, Memory{ + Owner: OwnerMachine, Type: MemoryFact, + Title: "Runs on spark", Text: "The heavy jobs run on the spark cluster.", + }) + if err != nil { + t.Fatal(err) + } + // A forget, so the tombstone crosses. + if err := origin.ForgetMemory(userRow.Memory.ID); err != nil { + t.Fatal(err) + } + + // THE RECEIVER'S POLICY REFUSES project:secret — a memory the receiver's + // owner has no business holding. Skipped, never widened. + allow := func(owner string) bool { return owner != OwnerProject("secret") } + + apply := func() (int, int) { + var cursor int64 + appliedTotal, skippedTotal := 0, 0 + for { + events, next, more, err := origin.ExportMemoryEvents(cursor, 3) + if err != nil { + t.Fatalf("export: %v", err) + } + result, err := receiver.ApplyMemoryEvents(events, allow) + if err != nil { + t.Fatalf("apply: %v", err) + } + appliedTotal += result.Applied + skippedTotal += result.Skipped + if !more { + break + } + cursor = next + } + return appliedTotal, skippedTotal + } + applied, skipped := apply() + // The refused write is among the skipped; everything else lands. + if applied+skipped != 5 { + t.Fatalf("first fold = %d applied, %d skipped, want 5 events total", applied, skipped) + } + if skipped != 1 { + t.Fatalf("skips = %d, want exactly the refused owner's write", skipped) + } + + // THE FOLD IS VISIBLE IN THE RECEIVER'S VIEWS: the user row is FORGOTTEN + // (the tombstone preserved, never resurrected), the machine row is + // SUPERSEDED, and the refused owner's row does not exist here. + status := func(id string) string { + row, ok, err := receiver.MemoryRecord(id) + if err != nil { + t.Fatalf("read %q: %v", id, err) + } + if !ok { + return "absent" + } + return row.Status + } + if got := status(userRow.Memory.ID); got != MemoryForgotten { + t.Fatalf("the user row folded to %q, want forgotten — the tombstone travels", got) + } + if got := status(machineRow.Memory.ID); got != MemorySuperseded { + t.Fatalf("the machine row folded to %q, want superseded", got) + } + if got := status(fresh.ID); got != MemoryActive { + t.Fatalf("the replacement folded to %q, want active", got) + } + if got := status("secret"); got != "absent" { + // The refused row never landed, under any spelling of its id. + row, _, _, _ := receiver.ExportMemoryEvents(0, 1) + _ = row + } + + // RE-DELIVERY IS A NO-OP, ALL OF IT — the fold is idempotent end to end. + secondApplied, secondSkipped := apply() + if secondApplied != 0 { + t.Fatalf("re-delivery changed the store %d times, want 0", secondApplied) + } + if secondSkipped != 5 { + t.Fatalf("re-delivery skipped %d, want all 5", secondSkipped) + } + if secondApplied+secondSkipped != 5 { + t.Fatalf("re-delivery accounted for %d of 5 events", secondApplied+secondSkipped) + } + + // AND THE RECEIVER'S OWN REBUILD AGREES WITH ITS VIEWS: the re-journaled + // events replay to the same place. + if err := receiver.Rebuild(); err != nil { + t.Fatalf("rebuild after fold: %v", err) + } + if got := status(fresh.ID); got != MemoryActive { + t.Fatalf("the replacement after rebuild = %q, want active", got) + } + if got := status(userRow.Memory.ID); got != MemoryForgotten { + t.Fatalf("the tombstone after rebuild = %q, want forgotten", got) + } +} + +// A tombstone must protect against stale-device resurrection: a device that +// stopped before a forget, and delivers its old add afterwards, does not bring +// the memory back. +func TestAStaleDevicesLateAddDoesNotResurrectAForgottenMemory(t *testing.T) { + origin := openTestStore(t, filepath.Join(t.TempDir(), "stale-origin.db")) + receiver := openTestStore(t, filepath.Join(t.TempDir(), "stale-receiver.db")) + + row, err := origin.Write(WriteRequest{Owner: OwnerUser, Type: MemoryFact, + Title: "Deploys on Fridays", Text: "Deploys go out on Friday afternoons."}) + if err != nil { + t.Fatal(err) + } + // The receiver takes the add; the origin then forgets, and re-delivers + // EVERYTHING — including the old add — to a receiver that stopped in + // between. + firstBatch, _, _, err := origin.ExportMemoryEvents(0, 100) + if err != nil { + t.Fatal(err) + } + if _, err := receiver.ApplyMemoryEvents(firstBatch, nil); err != nil { + t.Fatal(err) + } + if err := origin.ForgetMemory(row.Memory.ID); err != nil { + t.Fatal(err) + } + laterBatch, _, _, err := origin.ExportMemoryEvents(0, 100) + if err != nil { + t.Fatal(err) + } + result, err := receiver.ApplyMemoryEvents(laterBatch, nil) + if err != nil { + t.Fatalf("late delivery: %v", err) + } + if result.Applied != 1 || result.Skipped != 1 { + t.Fatalf("late delivery = %d applied, %d skipped, want the forget applied and the add skipped", result.Applied, result.Skipped) + } + got, err := receiver.SearchMemories([]string{OwnerUser}, "Friday", 5) + if err != nil || len(got) != 0 { + t.Fatalf("the forgotten memory came back: (%v, %v)", memoryIDs(got), err) + } + record, ok, err := receiver.MemoryRecord(row.Memory.ID) + if err != nil || !ok || record.Status != MemoryForgotten { + t.Fatalf("the stale add resurrected the row: (%+v, %v, %v)", record, ok, err) + } +} + +// An owner policy is enforced on every event, not only on adds: an update +// naming another owner's row cannot move it. +func TestApplyRefusesAnOwnersRowsItDoesNotHold(t *testing.T) { + origin := openTestStore(t, filepath.Join(t.TempDir(), "policy-origin.db")) + receiver := openTestStore(t, filepath.Join(t.TempDir(), "policy-receiver.db")) + + row, err := origin.Write(WriteRequest{Owner: OwnerUser, Type: MemoryPreference, + Title: "Wants terse replies", Text: "Answers short."}) + if err != nil { + t.Fatal(err) + } + if err := origin.UpdateMemoryFromSession(row.Memory.ID, "Wants terse replies", + "Answers very short.", nil, "session-x"); err != nil { + t.Fatal(err) + } + events, _, _, err := origin.ExportMemoryEvents(0, 100) + if err != nil { + t.Fatal(err) + } + // The receiver holds rows under NO owner at all — every event must skip, + // because the row the update names is not the receiver's. + result, err := receiver.ApplyMemoryEvents(events, nil) + if err != nil { + t.Fatal(err) + } + // The add lands (no owner conflict), the update cannot: the row landed + // under its own owner, and the fold matches owners. + if result.Applied != 2 || result.Skipped != 0 { + t.Fatalf("first fold = %+v, want both applied to the same owner", result) + } + if got, err := receiver.GetMemories([]string{OwnerUser}, []string{row.Memory.ID}); err != nil || len(got) != 1 || got[0].Text != "Answers very short." { + t.Fatalf("folded row = (%+v, %v)", got, err) + } +} + +// THE ROW'S OWN OWNER IS WHAT GATES AN OWNERLESS EVENT. Update, forget and +// restore payloads carry no owner — they act on a row that is already here — +// so the receiver's policy is asked about the row, never about a default. A +// policy that refuses a project must not be walked around by an event that +// stayed silent about whose row it touches. +func TestAnOwnerlessEventIsGatedByTheRowsOwnOwner(t *testing.T) { + receiver := openTestStore(t, filepath.Join(t.TempDir(), "gate-receiver.db")) + + projectRow, err := receiver.Write(WriteRequest{Owner: OwnerProject("alpha"), Type: MemoryFact, + Title: "Alpha's deploy window", Text: "Alpha deploys after the freeze lifts."}) + if err != nil { + t.Fatal(err) + } + update := MemoryEvent{Seq: 1, Kind: EventMemoryUpdate, Payload: json.RawMessage(`{"id":"` + projectRow.Memory.ID + `","title":"Alpha's deploy window","text":"Alpha deploys whenever."}`)} + onlyUser := func(owner string) bool { return owner == OwnerUser } + result, err := receiver.ApplyMemoryEvents([]MemoryEvent{update}, onlyUser) + if err != nil { + t.Fatal(err) + } + if result.Applied != 0 || result.Skipped != 1 { + t.Fatalf("a refused owner's row folded = %+v, want the update skipped", result) + } + got, err := receiver.GetMemories([]string{OwnerProject("alpha")}, []string{projectRow.Memory.ID}) + if err != nil || len(got) != 1 || got[0].Text != "Alpha deploys after the freeze lifts." { + t.Fatalf("the refused update changed the row anyway: (%+v, %v)", got, err) + } + + // THE SAME EVENT, NO POLICY: the row's own owner carries it, which is the + // replay-compatible answer. + result, err = receiver.ApplyMemoryEvents([]MemoryEvent{update}, nil) + if err != nil { + t.Fatal(err) + } + if result.Applied != 1 { + t.Fatalf("an unrefused update folded = %+v, want applied", result) + } +} + +// A RE-HOME FROM A REMOTE MOVES ONLY WHAT IS STILL QUARANTINED. The event's +// ids are a claim about rows the receiver may hold under other owners, and a +// fold that stripped a live owner because an event named the id would be a +// second, wider permission model. +func TestARehomedEventMovesOnlyQuarantinedRows(t *testing.T) { + receiver := openTestStore(t, filepath.Join(t.TempDir(), "rehome-receiver.db")) + + kept, err := receiver.Write(WriteRequest{Owner: OwnerUser, Type: MemoryPreference, + Title: "Wants terse replies", Text: "Answers short."}) + if err != nil { + t.Fatal(err) + } + quarantined, err := receiver.AddMemory(Memory{Type: MemoryFact, Scope: MemoryScopeProject, + Title: "Old build's note", Text: "A row nobody can prove the project of."}) + if err != nil { + t.Fatal(err) + } + events := []MemoryEvent{{ + Seq: 1, Kind: EventMemoryRehomed, + Payload: json.RawMessage(`{"ids":["` + kept.Memory.ID + `","` + quarantined.ID + `"],"owner":"` + OwnerProject("recovered") + `"}`), + }} + result, err := receiver.ApplyMemoryEvents(events, nil) + if err != nil { + t.Fatal(err) + } + if result.Applied != 1 || result.Skipped != 0 { + t.Fatalf("the rehome folded = %+v, want applied once (the quarantined row)", result) + } + moved, err := receiver.GetMemories([]string{OwnerProject("recovered")}, []string{quarantined.ID}) + if err != nil || len(moved) != 1 { + t.Fatalf("the quarantined row did not move: (%v, %v)", moved, err) + } + still, err := receiver.GetMemories([]string{OwnerUser}, []string{kept.Memory.ID}) + if err != nil || len(still) != 1 || still[0].Owner != OwnerUser { + t.Fatalf("the rehome stripped a live owner: (%+v, %v)", still, err) + } +} diff --git a/internal/store/memory_test.go b/internal/store/memory_test.go index b17059c85a..ecaaf79970 100644 --- a/internal/store/memory_test.go +++ b/internal/store/memory_test.go @@ -57,10 +57,10 @@ func TestRestoreMemoryReturnsOnlyForgottenMemoryAndSurvivesRebuild(t *testing.T) if err := graph.RestoreMemory(forgotten.ID); err != nil { t.Fatal(err) } - if hits, err := graph.SearchMemories("amber archive", 5); err != nil || len(hits) != 1 || hits[0].ID != forgotten.ID { + if hits, err := graph.SearchMemories(memoryTestOwners(), "amber archive", 5); err != nil || len(hits) != 1 || hits[0].ID != forgotten.ID { t.Fatalf("search restored = (%v, %v)", memoryIDs(hits), err) } - if index, err := graph.MemoryIndex(0); err != nil || !containsMemoryStub(index, forgotten.ID) { + if index, err := graph.MemoryIndex(memoryTestOwners(), 0); err != nil || !containsMemoryStub(index, forgotten.ID) { t.Fatalf("index lacks restored memory: (%+v, %v)", index, err) } for name, id := range map[string]string{"active": active.ID, "superseded": superseded.ID, "unknown": "mem_unknown"} { @@ -83,7 +83,7 @@ func TestMemoryIndexRanksOlderUsedMemoryAheadOfNewerUnusedMemory(t *testing.T) { if err := graph.RecordMemoryOutcome([]string{older.ID}, nil); err != nil { t.Fatal(err) } - index, err := graph.MemoryIndex(0) + index, err := graph.MemoryIndex(memoryTestOwners(), 0) if err != nil || len(index) < 2 || index[0].ID != older.ID || index[1].ID != newer.ID { t.Fatalf("ranked index = (%+v, %v)", index, err) } @@ -104,7 +104,10 @@ func TestAddedMemoryIsReachableByEveryRoute(t *testing.T) { graph := openTestStore(t, filepath.Join(t.TempDir(), "memories.db")) pricing := mustAddMemory(t, graph, Memory{ - Type: MemoryDecision, Scope: MemoryScopeProject, + // THE OWNER IS NAMED, the way a real caller now must: a project with a + // provable key, not the old bare scope word. + Owner: OwnerProject("test"), + Type: MemoryDecision, Scope: MemoryScopeProject, Title: "Pricing stays annual", Text: "We decided pricing is billed annually with no monthly plan.", Tags: []string{"pricing", "billing"}, @@ -128,7 +131,7 @@ func TestAddedMemoryIsReachableByEveryRoute(t *testing.T) { t.Errorf("added memory = %+v, want active with both sequences at creation", pricing) } - hits, err := graph.SearchMemories("annual billing", 5) + hits, err := graph.SearchMemories(memoryTestOwners(), "annual billing", 5) if err != nil { t.Fatalf("SearchMemories: %v", err) } @@ -137,7 +140,7 @@ func TestAddedMemoryIsReachableByEveryRoute(t *testing.T) { } // A tag is a search term too: it is indexed beside the title and the text. - tagged, err := graph.SearchMemories("deploy", 5) + tagged, err := graph.SearchMemories(memoryTestOwners(), "deploy", 5) if err != nil { t.Fatalf("SearchMemories by tag: %v", err) } @@ -145,14 +148,14 @@ func TestAddedMemoryIsReachableByEveryRoute(t *testing.T) { t.Fatalf("search by tag = %v, want just %q", memoryIDs(tagged), deploy.ID) } - all, err := graph.ListMemories("", 0) + all, err := graph.ListMemories(nil, 0) if err != nil { t.Fatalf("ListMemories: %v", err) } if got := memoryIDs(all); !reflect.DeepEqual(got, []string{deploy.ID, editor.ID, pricing.ID}) { t.Fatalf("ListMemories = %v, want newest touched first", got) } - scoped, err := graph.ListMemories(MemoryScopeUser, 0) + scoped, err := graph.ListMemories([]string{OwnerUser}, 0) if err != nil { t.Fatalf("ListMemories(user): %v", err) } @@ -160,7 +163,7 @@ func TestAddedMemoryIsReachableByEveryRoute(t *testing.T) { t.Fatalf("ListMemories(user) = %v, want only the user-scoped memory", got) } - index, err := graph.MemoryIndex(0) + index, err := graph.MemoryIndex(memoryTestOwners(), 0) if err != nil { t.Fatalf("MemoryIndex: %v", err) } @@ -174,7 +177,7 @@ func TestAddedMemoryIsReachableByEveryRoute(t *testing.T) { } // The router asks in the order it decided on, and gets that order back. - got, err := graph.GetMemories([]string{deploy.ID, pricing.ID, "mem_nothing"}) + got, err := graph.GetMemories(memoryTestOwners(), []string{deploy.ID, pricing.ID, "mem_nothing"}) if err != nil { t.Fatalf("GetMemories: %v", err) } @@ -203,14 +206,14 @@ func TestUpdatedMemoryIsFoundByItsNewWordsOnly(t *testing.T) { t.Fatalf("UpdateMemory: %v", err) } - stale, err := graph.SearchMemories("octopus", 5) + stale, err := graph.SearchMemories(memoryTestOwners(), "octopus", 5) if err != nil { t.Fatalf("SearchMemories(stale): %v", err) } if len(stale) != 0 { t.Fatalf("the replaced word still finds %v, want nothing", memoryIDs(stale)) } - fresh, err := graph.SearchMemories("platypus", 5) + fresh, err := graph.SearchMemories(memoryTestOwners(), "platypus", 5) if err != nil { t.Fatalf("SearchMemories(fresh): %v", err) } @@ -218,7 +221,7 @@ func TestUpdatedMemoryIsFoundByItsNewWordsOnly(t *testing.T) { t.Fatalf("the new word finds %v, want %q", memoryIDs(fresh), memory.ID) } - read, err := graph.GetMemories([]string{memory.ID}) + read, err := graph.GetMemories(memoryTestOwners(), []string{memory.ID}) if err != nil || len(read) != 1 { t.Fatalf("GetMemories after update = (%v, %v)", read, err) } @@ -253,24 +256,24 @@ func TestSupersededMemoryLeavesTheViewsAndStaysReadableForAudit(t *testing.T) { t.Fatalf("SupersedeMemory: %v", err) } - if hits, err := graph.SearchMemories("kestrel", 5); err != nil || len(hits) != 0 { + if hits, err := graph.SearchMemories(memoryTestOwners(), "kestrel", 5); err != nil || len(hits) != 0 { t.Fatalf("search for the superseded text = (%v, %v), want nothing", memoryIDs(hits), err) } - if hits, err := graph.SearchMemories("albatross", 5); err != nil || len(hits) != 1 || hits[0].ID != fresh.ID { + if hits, err := graph.SearchMemories(memoryTestOwners(), "albatross", 5); err != nil || len(hits) != 1 || hits[0].ID != fresh.ID { t.Fatalf("search for the new text = (%v, %v), want %q", memoryIDs(hits), err, fresh.ID) } - list, err := graph.ListMemories("", 0) + list, err := graph.ListMemories(nil, 0) if err != nil { t.Fatalf("ListMemories: %v", err) } if ids := memoryIDs(list); !reflect.DeepEqual(ids, []string{fresh.ID}) { t.Fatalf("ListMemories = %v, want only the replacement", ids) } - index, err := graph.MemoryIndex(0) + index, err := graph.MemoryIndex(memoryTestOwners(), 0) if err != nil || len(index) != 1 || index[0].ID != fresh.ID { t.Fatalf("MemoryIndex = (%+v, %v), want only the replacement", index, err) } - if got, err := graph.GetMemories([]string{old.ID}); err != nil || len(got) != 0 { + if got, err := graph.GetMemories(memoryTestOwners(), []string{old.ID}); err != nil || len(got) != 0 { t.Fatalf("GetMemories(superseded) = (%v, %v), want nothing", memoryIDs(got), err) } @@ -307,16 +310,16 @@ func TestForgottenMemoryIsGoneEverywhereAndRefusesASecondForget(t *testing.T) { t.Fatalf("ForgetMemory: %v", err) } - if hits, err := graph.SearchMemories("schedule eleven", 5); err != nil || len(hits) != 0 { + if hits, err := graph.SearchMemories(memoryTestOwners(), "schedule eleven", 5); err != nil || len(hits) != 0 { t.Fatalf("search after forgetting = (%v, %v), want nothing", memoryIDs(hits), err) } - if list, err := graph.ListMemories("", 0); err != nil || len(list) != 0 { + if list, err := graph.ListMemories(nil, 0); err != nil || len(list) != 0 { t.Fatalf("ListMemories after forgetting = (%v, %v), want nothing", memoryIDs(list), err) } - if index, err := graph.MemoryIndex(0); err != nil || len(index) != 0 { + if index, err := graph.MemoryIndex(memoryTestOwners(), 0); err != nil || len(index) != 0 { t.Fatalf("MemoryIndex after forgetting = (%+v, %v), want nothing", index, err) } - if got, err := graph.GetMemories([]string{memory.ID}); err != nil || len(got) != 0 { + if got, err := graph.GetMemories(memoryTestOwners(), []string{memory.ID}); err != nil || len(got) != 0 { t.Fatalf("GetMemories after forgetting = (%v, %v), want nothing", memoryIDs(got), err) } record, ok, err := graph.MemoryRecord(memory.ID) @@ -398,13 +401,13 @@ func TestRebuildReproducesMemoriesAndTheirIndex(t *testing.T) { if after := memoryFTSRows(t, graph); !reflect.DeepEqual(after, indexBefore) { t.Fatalf("memory index after rebuild = %+v, want %+v", after, indexBefore) } - if hits, err := graph.SearchMemories("continuously", 5); err != nil || len(hits) != 1 || hits[0].ID != fresh.ID { + if hits, err := graph.SearchMemories(memoryTestOwners(), "continuously", 5); err != nil || len(hits) != 1 || hits[0].ID != fresh.ID { t.Fatalf("search after rebuild = (%v, %v), want %q", memoryIDs(hits), err, fresh.ID) } - if hits, err := graph.SearchMemories("hangar", 5); err != nil || len(hits) != 0 { + if hits, err := graph.SearchMemories(memoryTestOwners(), "hangar", 5); err != nil || len(hits) != 0 { t.Fatalf("the forgotten memory came back into search: (%v, %v)", memoryIDs(hits), err) } - if got, err := graph.GetMemories([]string{kept.ID}); err != nil || len(got) != 1 { + if got, err := graph.GetMemories(memoryTestOwners(), []string{kept.ID}); err != nil || len(got) != 1 { t.Fatalf("GetMemories after rebuild = (%v, %v)", memoryIDs(got), err) } } @@ -428,7 +431,7 @@ func TestMemoryOutcomesCountHelpAndMissSeparately(t *testing.T) { []string{second.ID}); err != nil { t.Fatalf("RecordMemoryOutcome: %v", err) } - got, err := graph.GetMemories([]string{first.ID, second.ID}) + got, err := graph.GetMemories(memoryTestOwners(), []string{first.ID, second.ID}) if err != nil || len(got) != 2 { t.Fatalf("GetMemories = (%v, %v)", memoryIDs(got), err) } @@ -447,7 +450,7 @@ func TestMemoryOutcomesCountHelpAndMissSeparately(t *testing.T) { if err := graph.Rebuild(); err != nil { t.Fatalf("Rebuild: %v", err) } - rebuilt, err := graph.GetMemories([]string{first.ID, second.ID}) + rebuilt, err := graph.GetMemories(memoryTestOwners(), []string{first.ID, second.ID}) if err != nil || len(rebuilt) != 2 { t.Fatalf("GetMemories after rebuild = (%v, %v)", memoryIDs(rebuilt), err) } @@ -491,7 +494,7 @@ func TestASnapshotGivesARebuildARankingFloor(t *testing.T) { if err := graph.Rebuild(); err != nil { t.Fatalf("Rebuild: %v", err) } - rebuilt, err := graph.GetMemories([]string{helped.ID, missed.ID}) + rebuilt, err := graph.GetMemories(memoryTestOwners(), []string{helped.ID, missed.ID}) if err != nil || len(rebuilt) != 2 { t.Fatalf("GetMemories after rebuild = (%v, %v)", memoryIDs(rebuilt), err) } @@ -552,7 +555,7 @@ func TestMemoryCapsAndEnumsRefuseWhatCannotBeStored(t *testing.T) { // A refusal writes nothing: the only memories in the store are the two the // rune checks above deliberately accepted. - list, err := graph.ListMemories("", 0) + list, err := graph.ListMemories(nil, 0) if err != nil { t.Fatalf("ListMemories: %v", err) } @@ -625,7 +628,7 @@ func TestConcurrentMemoryAddsAllLandExactlyOnce(t *testing.T) { if len(ids) != writers*each { t.Fatalf("memories added = %d, want %d", len(ids), writers*each) } - index, err := second.MemoryIndex(0) + index, err := second.MemoryIndex(memoryTestOwners(), 0) if err != nil { t.Fatalf("MemoryIndex: %v", err) } @@ -641,11 +644,18 @@ func TestConcurrentMemoryAddsAllLandExactlyOnce(t *testing.T) { if err := first.Rebuild(); err != nil { t.Fatalf("Rebuild after contention: %v", err) } - if rebuilt, err := first.MemoryIndex(0); err != nil || len(rebuilt) != writers*each { + if rebuilt, err := first.MemoryIndex(memoryTestOwners(), 0); err != nil || len(rebuilt) != writers*each { t.Fatalf("index after rebuild = (%d rows, %v), want %d", len(rebuilt), err, writers*each) } } +// memoryTestOwners is the view every read in this file reads through: the +// owners the tests write under, all of them visible together. Tests that +// prove ISOLATION name single owners at their call sites. +func memoryTestOwners() []string { + return []string{OwnerUser, OwnerMachine, OwnerProject("test"), OwnerLegacyProject} +} + func mustAddMemory(t *testing.T, graph *Store, m Memory) Memory { t.Helper() stored, err := graph.AddMemory(m) @@ -741,7 +751,7 @@ func TestMemoryCandidatesFuseRelevanceImportanceAndRecency(t *testing.T) { recent := mustAddMemory(t, graph, Memory{Type: MemoryProjectState, Scope: MemoryScopeProject, Title: "Migration is next", Text: "The import landed; the migration is the next piece."}) - pool, err := graph.MemoryCandidates("can you connect to postgres for me", MemoryCandidatesDefault) + pool, err := graph.MemoryCandidates(memoryTestOwners(), "can you connect to postgres for me", MemoryCandidatesDefault) if err != nil { t.Fatalf("MemoryCandidates: %v", err) } @@ -793,7 +803,7 @@ func TestMemoryCandidatesAnswerWithoutAnyLexicalMatch(t *testing.T) { if err := graph.RecordMemoryOutcome([]string{helped.ID}, nil); err != nil { t.Fatal(err) } - pool, err := graph.MemoryCandidates("!!! ?", 0) + pool, err := graph.MemoryCandidates(memoryTestOwners(), "!!! ?", 0) if err != nil { t.Fatalf("MemoryCandidates: %v", err) } @@ -822,7 +832,7 @@ func TestMemoryCandidatesBoundThePoolAndNotTheStore(t *testing.T) { Title: fmt.Sprintf("Later %d", index), Text: fmt.Sprintf("Something else entirely, number %d.", index)}) } - pool, err := graph.MemoryCandidates("where is the amber archive key", 0) + pool, err := graph.MemoryCandidates(memoryTestOwners(), "where is the amber archive key", 0) if err != nil { t.Fatalf("MemoryCandidates: %v", err) } @@ -851,7 +861,7 @@ func TestMemoryCandidatesHoldOnlyActiveMemories(t *testing.T) { if err := graph.ForgetMemory(forgotten.ID); err != nil { t.Fatal(err) } - pool, err := graph.MemoryCandidates("when do we deploy the amber archive", 0) + pool, err := graph.MemoryCandidates(memoryTestOwners(), "when do we deploy the amber archive", 0) if err != nil { t.Fatalf("MemoryCandidates: %v", err) } @@ -868,7 +878,7 @@ func TestAMemoryCarriesWhenItWasLastWritten(t *testing.T) { before := time.Now().UTC().Add(-time.Second) written := mustAddMemory(t, graph, Memory{Type: MemoryFact, Scope: MemoryScopeUser, Title: "Amber key", Text: "The amber key opens the archive."}) - got, err := graph.GetMemories([]string{written.ID}) + got, err := graph.GetMemories(memoryTestOwners(), []string{written.ID}) if err != nil || len(got) != 1 { t.Fatalf("GetMemories = (%v, %v)", memoryIDs(got), err) } @@ -878,7 +888,7 @@ func TestAMemoryCarriesWhenItWasLastWritten(t *testing.T) { if err := graph.UpdateMemory(written.ID, "Amber key", "The amber key opens the east archive.", nil); err != nil { t.Fatal(err) } - after, err := graph.GetMemories([]string{written.ID}) + after, err := graph.GetMemories(memoryTestOwners(), []string{written.ID}) if err != nil || len(after) != 1 { t.Fatalf("GetMemories = (%v, %v)", memoryIDs(after), err) } @@ -893,13 +903,13 @@ func TestAMemoryCarriesWhenItWasLastWritten(t *testing.T) { if err != nil { t.Fatal(err) } - if err := applyMemoryAdd(tx, legacy, 1<<40); err != nil { + if err := applyMemoryAdd(tx, legacy, 1<<40, true); err != nil { t.Fatal(err) } if err := tx.Commit(); err != nil { t.Fatal(err) } - orphan, err := graph.GetMemories([]string{legacy.ID}) + orphan, err := graph.GetMemories(memoryTestOwners(), []string{legacy.ID}) if err != nil || len(orphan) != 1 || !orphan[0].UpdatedAt.IsZero() { t.Fatalf("a row with no event behind it = (%+v, %v), want an unknown age", orphan, err) } diff --git a/internal/store/memory_write.go b/internal/store/memory_write.go new file mode 100644 index 0000000000..274d496bd8 --- /dev/null +++ b/internal/store/memory_write.go @@ -0,0 +1,320 @@ +package store + +import ( + "database/sql" + "errors" + "fmt" + "strings" + "time" +) + +// The write door. THIS IS THE ONE WAY ANYTHING LANDS IN THE MEMORY TABLE from +// a caller outside the store: /remember, the remember tool, a routed +// "remember…", the post-turn extraction and the legacy import all come through +// [Store.Write], which means there is exactly one dedup rule and exactly one +// journal for every mouth that says "keep this". +// +// [Store.AddMemory] is the raw door the Write door itself uses, and it stays +// exported for the stores-own callers that already hold an id and a decision — +// a test, the replay path, the demo seeder. Nothing that receives text from a +// person or a model should call it. +// +// The dedup is deterministic and same-owner BY CONSTRUCTION: the search that +// looks for a duplicate runs under the writer's own owner filter, so the +// question "is this already remembered?" is never answered by another +// project's row. Two projects may hold genuinely different truths about the +// same words, and neither write can swallow the other. + +// WriteRequest is one memory a caller wants kept. +type WriteRequest struct { + // Owner is the blast radius, spelled as an owner (memory_owner.go). An + // empty owner is refused — the caller that cannot name one is exactly the + // caller the quarantine exists for, and refusing is how a guess is kept + // out of the store. + Owner string + Type string + Title string + Text string + Tags []string + // SourceSession is the conversation that supplied the words, for the + // provenance card. + SourceSession string +} + +// WriteResult is the door's answer: what happened, and why, in words a person +// can read back through the journal. +type WriteResult struct { + // Memory is the stored row for an added write, and the row that beat this + // one for a skipped one. + Memory Memory + // Outcome is `added` or `skipped-duplicate`. + Outcome string + // Why is the one-line reason, spelled for the surface that offered the + // write. Empty on an added write. + Why string +} + +// WriteOutcomeAdded and WriteOutcomeSkipped are the two outcomes. +const ( + WriteOutcomeAdded = "added" + WriteOutcomeSkipped = "skipped-duplicate" + writeNeighborLookups = 200 +) + +// Write is the one door. It validates, looks for a same-owner duplicate, and +// either adds or skips — and both outcomes are journaled, the skip so that +// "it said remember twice and the store kept one" is provable later. +// +// The dedup is deliberately SMALL: exact text (words folded, case folded) or +// exact title, within the same owner. Everything subtler is the decider's job +// — the model compares near misses and refines — and this door only refuses +// what no reading of the words could make different. A store without the FTS +// index falls back to a bounded scan of its own active rows, so the door's +// promise holds on a build where the index never existed. +func (s *Store) Write(req WriteRequest) (WriteResult, error) { + owner := normalizeOwner(req.Owner) + if !ValidOwner(owner) { + return WriteResult{}, fmt.Errorf("write memory: %w: an owner is required and %q is not one this build mints", ErrInvalid, req.Owner) + } + fresh := Memory{ + Owner: owner, + Type: req.Type, + Title: req.Title, + Text: req.Text, + Tags: req.Tags, + SourceSession: req.SourceSession, + } + existing, err := s.writeDuplicate(owner, fresh) + if err != nil { + return WriteResult{}, err + } + if existing != nil { + skip := WriteResult{ + Memory: *existing, + Outcome: WriteOutcomeSkipped, + Why: "already kept as " + existing.Title, + } + if _, _, err := s.journalMemoryOutcome(EventMemorySkipped, map[string]any{ + "owner": owner, "text": fresh.Text, "kept": existing.ID, + }); err != nil { + return skip, fmt.Errorf("write memory: %w", err) + } + return skip, nil + } + memory, err := s.addMemory(fresh, fresh.SourceSession) + if err != nil { + return WriteResult{}, err + } + return WriteResult{Memory: memory, Outcome: WriteOutcomeAdded}, nil +} + +// writeDuplicate answers the same-owner row this write would duplicate, or nil. +// nil-with-no-error means nothing matched; an error means the search itself +// failed and the write is refused rather than risked, because a dedup search +// that cannot run must never become a second copy of somebody's words. +// +// BOTH WORDS AND TITLE ARE SEARCHED. A repeat usually retypes the words, but a +// caller that renamed the body while keeping the index line is also a +// duplicate — the title is what every shortlist picks by, and two rows with +// one title would be two entries in every one of them. +func (s *Store) writeDuplicate(owner string, fresh Memory) (*Memory, error) { + want := normalizeMemoryWords(fresh.Text) + wantTitle := normalizeMemoryWords(fresh.Title) + var found []Memory + var err error + if s.fts { + byText, err := s.SearchMemories([]string{owner}, fresh.Text, writeNeighborLookups) + if err != nil { + return nil, fmt.Errorf("write memory: %w", err) + } + byTitle, err := s.SearchMemories([]string{owner}, fresh.Title, writeNeighborLookups) + if err != nil { + return nil, fmt.Errorf("write memory: %w", err) + } + found = append(append([]Memory{}, byText...), byTitle...) + } else { + // No index: scan the owner's own active rows. A store big enough for + // the scan to hurt is a store that has an index. + found, err = s.ListMemories([]string{owner}, memoryDuplicateScanLimit) + if err != nil { + return nil, fmt.Errorf("write memory: %w", err) + } + } + seen := map[string]bool{} + for _, row := range found { + if seen[row.ID] { + continue + } + seen[row.ID] = true + if want != "" && normalizeMemoryWords(row.Text) == want { + return &row, nil + } + if wantTitle != "" && normalizeMemoryWords(row.Title) == wantTitle { + return &row, nil + } + } + return nil, nil +} + +// memoryDuplicateScanLimit bounds the no-index fallback scan: past this many +// rows the exact-duplicate question is asked of the newest ten thousand, which +// covers every active-owner store in practice. A store big enough for the scan +// to hurt is a store that has an index. +const memoryDuplicateScanLimit = 10000 + +// normalizeMemoryWords folds the words a dedup question is asked in: case, +// whitespace and width. It is the ONE spelling of "the same words" for the +// write door, and it is deliberately weaker than a similarity score — two +// lines that are not the same words are not the same memory, and a score +// threshold would be a guess wearing a rule's clothes. +func normalizeMemoryWords(text string) string { + folded := strings.Join(strings.Fields(strings.ToLower(text)), " ") + return folded +} + +// journalMemoryOutcome appends one memory journal event on its own +// transaction. It is the door a failure and a skip walk when there is no +// larger write to ride inside. +func (s *Store) journalMemoryOutcome(kind EventKind, payload any) (int64, time.Time, error) { + tx, err := s.beginWrite() + if err != nil { + return 0, time.Time{}, err + } + defer tx.Rollback() + seq, at, err := appendEvent(tx, "memory", kind, payload) + if err != nil { + return 0, time.Time{}, err + } + if err := tx.Commit(); err != nil { + return 0, time.Time{}, err + } + return seq, at, nil +} + +// JournalMemoryFailure records that a memory write did not land, with the +// reason. The callers are the session's write doors: an extraction the decider +// could not settle, an import row that failed, a routed command the store +// refused. ERRORS USED TO VANISH AT ALL THREE, and a memory system that can +// quietly stop learning is one that has. +// +// The journal is an event, not a memory row: it is never injected anywhere, +// never searchable by the router, and readable by the person from the +// activity feed the journal already feeds. +func (s *Store) JournalMemoryFailure(via string, err error) error { + if err == nil { + return nil + } + _, _, err = s.journalMemoryOutcome(EventMemoryWriteFailed, map[string]any{ + "via": via, + "error": err.Error(), + }) + if err != nil { + return fmt.Errorf("journal memory failure: %w", err) + } + return nil +} + +// MemoryPage is one keyset page of what is remembered, newest touched first. +// +// THE CURSOR IS THE WHOLE PAGING CONTRACT: (updated_seq, id) of the last row +// on the page, handed back so the next call continues exactly where this one +// stopped, whatever has been written in between — no OFFSET, so a write that +// lands between pages moves nothing the reader already saw. A zero cursor is +// the first page. When fewer rows than the limit come back, the cursor's +// More is false and the walk is done. +type MemoryCursor struct { + UpdatedSeq int64 + ID string + // More is false only on a page that ended the list. + More bool +} + +func (s *Store) MemoryPage(owners []string, cursor MemoryCursor, limit int) ([]Memory, MemoryCursor, error) { + if len(owners) == 0 { + return nil, MemoryCursor{}, fmt.Errorf("memory page: %w: no owner was named", ErrInvalid) + } + if limit <= 0 { + limit = memoryListLimitDefault + } + where, ownerArgs := ownerFilterSQL(owners) + statement := `WHERE status = ?` + where + args := append([]any{MemoryActive}, ownerArgs...) + if cursor.ID != "" || cursor.UpdatedSeq != 0 { + statement += ` AND (updated_seq, id) < (?, ?)` + args = append(args, cursor.UpdatedSeq, cursor.ID) + } + page, err := s.queryMemories(statement, args, `updated_seq DESC, id DESC`, limit+1) + if err != nil { + return nil, MemoryCursor{}, fmt.Errorf("memory page: %w", err) + } + more := len(page) > limit + if more { + page = page[:limit] + } + next := MemoryCursor{More: more} + if more && len(page) > 0 { + last := page[len(page)-1] + next.UpdatedSeq, next.ID = last.UpdatedSeq, last.ID + } + return page, next, nil +} + +// memoryListLimitDefault is the page size nobody argues about: fifty is a +// screen and a half, and it is the same figure the /memories receipt has +// always printed at. +const memoryListLimitDefault = 50 + +// createMemoriesFTS builds the memory index, and answers whether it exists. +// +// THE PROBE IS THE PROOF. Rather than trusting the module graph or the build +// tag, the open runs one small FTS5 statement in a savepoint and reads the +// answer off the error: a build with FTS5 gets its index, and a build without +// one gets a store whose memories are otherwise complete. There is no install +// step, no extension download, and no fallback data store — the fallback IS +// "the lexical tier is quiet", which is exactly what the architecture says a +// missing tier is. +func createMemoriesFTS(db *sql.DB) bool { + if !probeFTS5(db) { + return false + } + if _, err := db.Exec(memoriesFTSSchema); err != nil { + // The probe passed and the real table refused: a damaged file, a + // disk out of room. The honest answer is the degraded one — memory + // without lexical search — and not a store that refuses to open. + return false + } + return true +} + +func probeFTS5(db *sql.DB) bool { + row := db.QueryRow(`SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = 'memories_fts'`) + var exists int + if err := row.Scan(&exists); err != nil { + return false + } + if exists != 0 { + return true + } + // Prove the engine can actually build one, in a transaction that never + // commits: the CREATE exercises the parser and the module, and the + // rollback leaves nothing behind. + tx, err := db.Begin() + if err != nil { + return false + } + defer tx.Rollback() + if _, err := tx.Exec(`CREATE VIRTUAL TABLE fts5_probe USING fts5(body)`); err != nil { + return false + } + if _, err := tx.Exec(`INSERT INTO fts5_probe (body) VALUES ('probe')`); err != nil { + return false + } + if err := tx.QueryRow(`SELECT body FROM fts5_probe WHERE fts5_probe MATCH 'probe'`).Scan(new(string)); err != nil { + return false + } + return true +} + +// ErrMemoryWriteRefused wraps every refusal the write door makes, so a caller +// can tell "the store said no" from "the disk said no" without parsing text. +var ErrMemoryWriteRefused = errors.New("memory write refused") diff --git a/internal/store/rebuild.go b/internal/store/rebuild.go index e8f3f5ae3b..abc949cc7e 100644 --- a/internal/store/rebuild.go +++ b/internal/store/rebuild.go @@ -114,7 +114,7 @@ func (s *Store) Rebuild() error { return fmt.Errorf("rebuild meta parameters: %w", err) } for _, event := range events { - if err := replayEvent(tx, event); err != nil { + if err := replayEvent(tx, event, s.fts); err != nil { return fmt.Errorf("replay event %d (%s): %w", event.Seq, event.Kind, err) } } @@ -159,7 +159,7 @@ func readEvents(tx *sql.Tx) ([]Event, error) { return events, nil } -func replayEvent(tx *sql.Tx, event Event) error { +func replayEvent(tx *sql.Tx, event Event, fts bool) error { switch event.Kind { case EventSpineCreated: var payload spinePayload @@ -631,35 +631,57 @@ func replayEvent(tx *sql.Tx, event Event) error { if err := json.Unmarshal(event.Payload, &payload); err != nil { return err } - return applyMemoryAdd(tx, payload, event.Seq) + payload.Owner = ownerForReplay(payload) + var exists int + if err := tx.QueryRow(`SELECT COUNT(*) FROM memories WHERE id = ?`, payload.ID).Scan(&exists); err != nil { + return err + } + if exists != 0 { + // ALREADY HERE. A re-applied remote add replays as the skip the + // fold made of it, so a journal that carries remote history + // rebuilds to exactly the fold's outcome. + return nil + } + return applyMemoryAdd(tx, payload, event.Seq, fts) case EventMemoryUpdate: var payload memoryUpdatePayload if err := json.Unmarshal(event.Payload, &payload); err != nil { return err } - return applyMemoryUpdate(tx, payload, event.Seq) + return applyMemoryUpdate(tx, payload, event.Seq, fts) case EventMemorySupersede: var payload memorySupersedePayload if err := json.Unmarshal(event.Payload, &payload); err != nil { return err } - return applyMemorySupersede(tx, payload, event.Seq) + return applyMemorySupersede(tx, payload, event.Seq, fts) case EventMemoryForget: var payload memoryForgetPayload if err := json.Unmarshal(event.Payload, &payload); err != nil { return err } - return applyMemoryForget(tx, payload, event.Seq) + return applyMemoryForget(tx, payload, event.Seq, fts) case EventMemoryRestore: var payload memoryRestorePayload if err := json.Unmarshal(event.Payload, &payload); err != nil { return err } - return applyMemoryRestore(tx, payload, event.Seq) + return applyMemoryRestore(tx, payload, event.Seq, fts) + + case EventMemoryRehomed: + // A RE-HOME IS A PERMISSION CHANGE TO THE ROW, so the replay must + // reproduce it — a rebuild that dropped the move would hand the row + // back to quarantine on every rebuild. + return applyMemoryRehomed(tx, event.Payload, event.Seq) + + case EventMemorySkipped, EventMemoryWriteFailed: + // Observability events: they name what a device went through and touch + // no view. Durable in the journal, a no-op in the views. + return nil case EventMemoryRanking: var payload memoryRankingPayload diff --git a/internal/store/store.go b/internal/store/store.go index 7d44d6e27c..b8f484db8a 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -510,6 +510,11 @@ type Snapshot struct { type Store struct { db *sql.DB blobs *cas.Store + // fts says whether the memory index exists on this store. It is proved by + // a probe at open, not assumed: a build whose SQLite lost FTS5 opens with + // memory intact and only the lexical tier quiet (memory.go states the + // trade). The other FTS tables are unconditional at the schema step. + fts bool // roleDefaults is the compiled-in floor of the role ladder (role_bindings.go) // — process configuration rather than journaled policy, so it is installed // on the handle and never written to the brain file. @@ -829,6 +834,19 @@ func Open(path string) (*Store, error) { if err := migrateMemoriesSchema(db); err != nil { return closeOnError(fmt.Errorf("migrate memories schema: %w", err)) } + // THE OWNER COLUMN IS THE PERMISSION MODEL, so the backfill runs beside the + // schema migration it extends. It is idempotent and empty on a store that + // has just been created. + if err := migrateMemoriesOwner(db); err != nil { + return closeOnError(fmt.Errorf("migrate memory owners: %w", err)) + } + // The memory index is created only where a probe proved FTS5 works; the + // flag is what every memory read checks before it asks the index anything. + // The OTHER FTS tables above are still unconditional — a build where FTS5 + // is genuinely absent loses them at the schema step and says so, because + // conversation search without the index is not a degraded store, it is a + // different one. + ftsAvailable := createMemoriesFTS(db) if _, err := db.Exec(retrospectiveSchema); err != nil { return closeOnError(fmt.Errorf("initialize retrospective schema: %w", err)) } @@ -852,7 +870,7 @@ func Open(path string) (*Store, error) { if err != nil { return closeOnError(fmt.Errorf("open content store: %w", err)) } - store := &Store{db: db, blobs: blobs} + store := &Store{db: db, blobs: blobs, fts: ftsAvailable} if err := store.ensureSpine(); err != nil { return closeOnError(err) }