diff --git a/LifeOS/install/LIFEOS/DOCUMENTATION/Memory/MemorySystem.md b/LifeOS/install/LIFEOS/DOCUMENTATION/Memory/MemorySystem.md index 88192639be..8759c912b9 100755 --- a/LifeOS/install/LIFEOS/DOCUMENTATION/Memory/MemorySystem.md +++ b/LifeOS/install/LIFEOS/DOCUMENTATION/Memory/MemorySystem.md @@ -208,6 +208,8 @@ The reviewer runs in **curation mode**: it reads the file's CURRENT entries (`re set-overwrite has a whole-file blast radius. `MemoryWriter.setEntries` computes a **catastrophic-shrink guard IN-LOCK** (against the just-read prior state, race-free): it blocks a result that is near-empty (`< 3` entries) OR a mass deletion (`>50%` dropped) **with zero additions**, while ALLOWING honest large consolidation (many drops accompanied by additions). `ESUSPECT_SHRINK` is the error; `allowDrastic` opts out for legitimate restores. (This guard exists because a cross-vendor audit wiped the live file with an empty `op:"set"` — it's now structurally impossible.) A second in-lock guard catches **slow erosion** (public issue #1761, @jacobo-ortiz): a single write that net-drops 2+ entries returns `ESUSPECT_EROSION` and logs the full delta to the write-log — re-transcription bleed passed the catastrophic checks every time (each write ~10% smaller, each carrying additions) until 12 durable rules died in 48h on a reporter's install. Deliberate consolidation passes with `allowDrastic: true`. +**Stale-base guard.** The reviewer's list is computed from a snapshot taken before minutes of inference, so a write landing in between (a manual curation pass, another writer) would be silently undone — and when the stale list is the larger one, neither guard above sees it. `read()` returns a `fingerprint` (sha256 of the valid entries); the reviewer passes its snapshot's fingerprint through `MemorySystem.add(item, { expectedFingerprint })` — an argument, never an item field, since items are model-facing — and `setEntries` recomputes it in-lock, refusing with `ESTALE_BASE` when the entries changed (frontmatter-only changes don't count). `op:"add"` guards with its own read; `op:"set"` callers without a fingerprint are unguarded, as before. The reviewer counts a refusal as a guard skip and retries at the next review. Refusals and lock timeouts (`ELOCK_HELD`, after a 2 s retry) are logged to `memory-writes.jsonl`, and `MemoryHealthCheck` reports them as `refused-writes` for 24h. + ### Per-write snapshots (recoverability) Every Tier-A write calls `snapshotBeforeWrite` — the prior file content is copied to a ring buffer at `MEMORY/OBSERVABILITY/memory-snapshots/__.md` (last 30 per file) before the overwrite. So any individual autonomic write is reversible — not just at git-commit granularity. Recover via `bun LIFEOS/TOOLS/MemoryRestore.ts {list | restore | latest }`. diff --git a/LifeOS/install/LIFEOS/TOOLS/MemoryHealthCheck.ts b/LifeOS/install/LIFEOS/TOOLS/MemoryHealthCheck.ts index a46c7b26c5..59bfc7a17f 100755 --- a/LifeOS/install/LIFEOS/TOOLS/MemoryHealthCheck.ts +++ b/LifeOS/install/LIFEOS/TOOLS/MemoryHealthCheck.ts @@ -345,6 +345,35 @@ if (existsSync(REVIEWER_RUNS)) { } catch { /* non-fatal */ } } +// CHECK 10: refused writes — a write dropped because the file changed after its +// base was read (ESTALE_BASE) or the lock never cleared (ELOCK_HELD). The +// reviewer's stdio is ignored, so this log is the only place a refusal shows up. +// Warns for 24h, then clears on its own; a steady stream means the reviewer keeps +// losing races and its read→write window needs redesign. +{ + const WRITES_FILE = join(OBS_DIR, "memory-writes.jsonl"); + const nowMs = process.env.CORTEX_HEALTH_NOW ? Date.parse(process.env.CORTEX_HEALTH_NOW) : Date.now(); + try { + // Whole file, filtered by time: a row count would let a busy day push a + // refusal out early. A future timestamp (clock skew) is not counted. + const refused = existsSync(WRITES_FILE) + ? readFileSync(WRITES_FILE, "utf-8").trim().split("\n") + .map(l => { try { return JSON.parse(l); } catch { return null; } }) + .filter((r: any) => { + if (r?.rejected !== true || (r.rejection_code !== "ESTALE_BASE" && r.rejection_code !== "ELOCK_HELD")) return false; + const age = nowMs - Date.parse(r.ts); + return age >= 0 && age < 24 * 60 * 60 * 1000; + }) + : []; + if (refused.length > 0) { + add("refused-writes", "warn", `${refused.length} memory write${refused.length === 1 ? "" : "s"} refused in 24h (stale or locked)`, + { rows: refused.map((r: any) => ({ ts: r.ts, file: r.file, code: r.rejection_code, by: r.updated_by })) }); + } else { + add("refused-writes-none", "ok", "No memory writes refused in the last 24h."); + } + } catch { /* non-fatal */ } +} + // F5: evidence-driven Cortex health. Paths and clock are injectable so tests never touch live state. function cortexThresholdEnv(name: string): number | undefined { const raw = process.env[name]; if (raw === undefined || raw === "") return undefined; diff --git a/LifeOS/install/LIFEOS/TOOLS/MemoryReviewer.ts b/LifeOS/install/LIFEOS/TOOLS/MemoryReviewer.ts index d350d275e8..15a9b0d73e 100755 --- a/LifeOS/install/LIFEOS/TOOLS/MemoryReviewer.ts +++ b/LifeOS/install/LIFEOS/TOOLS/MemoryReviewer.ts @@ -326,6 +326,8 @@ A confident "nothing to save" is correct.`; export interface CurrentMemorySnapshot { principal: string[]; assistant: string[]; + /** Fingerprint of each list as read; absent when the file could not be read. */ + fingerprints?: { principal?: string; assistant?: string }; } const PRINCIPAL_MEMORY_PATH = pathResolve(CLAUDE_ROOT, "LIFEOS/USER/PRINCIPAL/PRINCIPAL_MEMORY.md"); @@ -333,11 +335,16 @@ const DA_MEMORY_PATH = pathResolve(CLAUDE_ROOT, "LIFEOS/USER/DIGITAL_ASSISTANT/D /** Read both hot-layer files' current entries so the reviewer curates against reality. */ export function readCurrentMemorySnapshot(): CurrentMemorySnapshot { - const readEntries = (path: string): string[] => { - const r = memoryWriterRead(path); - return "code" in r ? [] : r.entries; + const principal = memoryWriterRead(PRINCIPAL_MEMORY_PATH); + const assistant = memoryWriterRead(DA_MEMORY_PATH); + return { + principal: "code" in principal ? [] : principal.entries, + assistant: "code" in assistant ? [] : assistant.entries, + fingerprints: { + principal: "code" in principal ? undefined : principal.fingerprint, + assistant: "code" in assistant ? undefined : assistant.fingerprint, + }, }; - return { principal: readEntries(PRINCIPAL_MEMORY_PATH), assistant: readEntries(DA_MEMORY_PATH) }; } function renderCurrentMemory(snap: CurrentMemorySnapshot | undefined): string[] { @@ -493,10 +500,13 @@ export interface DispatchSummary { /** A deliberate MemoryWriter safety refusal (net-drop erosion block), not a pipeline fault. */ function isGuardRefusal(result: AddResult): boolean { - return !result.ok && /ESUSPECT_EROSION/.test((result as { message?: string }).message ?? ""); + return !result.ok && /ESUSPECT_EROSION|ESTALE_BASE/.test((result as { message?: string }).message ?? ""); } -export function dispatchItems(items: TypedItem[], opts: { dryRun?: boolean; confidenceThreshold?: number } = {}): { summary: DispatchSummary; results: AddResult[] } { +export function dispatchItems( + items: TypedItem[], + opts: { dryRun?: boolean; confidenceThreshold?: number; baseFingerprints?: CurrentMemorySnapshot["fingerprints"] } = {}, +): { summary: DispatchSummary; results: AddResult[] } { const summary: DispatchSummary = { total: items.length, by_type: {}, @@ -510,6 +520,7 @@ export function dispatchItems(items: TypedItem[], opts: { dryRun?: boolean; conf }; const results: AddResult[] = []; const threshold = opts.confidenceThreshold ?? loadConfidenceThreshold(); + const bases: NonNullable = { ...opts.baseFingerprints }; for (let i = 0; i < items.length; i++) { const item = items[i]; @@ -521,8 +532,15 @@ export function dispatchItems(items: TypedItem[], opts: { dryRun?: boolean; conf continue; } - const result = memoryAdd(item); + // Memory sets replace the whole file with a list computed from the snapshot + // taken before inference; the fingerprint makes the write refuse if the file + // changed in the meantime. It comes from our own read, never from the model. + const result = memoryAdd(item, item.type === "memory" ? { expectedFingerprint: bases[item.actor] } : {}); results.push(result); + // A second item for the same actor builds on this write, not the old snapshot. + if (item.type === "memory" && result.ok && typeof result.detail?.fingerprint === "string") { + bases[item.actor] = result.detail.fingerprint; + } if (result.ok) { summary.succeeded++; @@ -575,7 +593,8 @@ export function dispatchItems(items: TypedItem[], opts: { dryRun?: boolean; conf } } else if (isGuardRefusal(result)) { // A safety-guard refusal (ESUSPECT_EROSION — the MemoryWriter blocking a net-drop - // consolidation) is the guard working, not a pipeline failure. Counting it as `failed` + // consolidation; ESTALE_BASE — the file changed during inference, so a newer edit + // won; MemoryHealthCheck reports those separately) is the guard working, not a pipeline failure. Counting it as `failed` // flips the run ok=false and trips the memory-health CRITICAL alert on a correct refusal // (same class as the empty-transcript→skipped precedent). Surface it as a skip: the fuller // memory set the guard preserved is intact, and retrying with allowDrastic is a human call. @@ -731,7 +750,7 @@ export async function review(opts: ReviewOptions = {}): Promise { writeRunDebug(runId, { "response.parsed.json": JSON.stringify(parsed.output, null, 2) }); // 6. Dispatch - const { summary, results } = dispatchItems(parsed.output.items, { dryRun: opts.dryRun }); + const { summary, results } = dispatchItems(parsed.output.items, { dryRun: opts.dryRun, baseFingerprints: snapshot.fingerprints }); writeRunDebug(runId, { "dispatch.log": [ `Items: ${summary.total} (succeeded=${summary.succeeded} failed=${summary.failed} skipped_guard=${summary.skipped_guard})`, diff --git a/LifeOS/install/LIFEOS/TOOLS/MemoryStaleWrite.test.ts b/LifeOS/install/LIFEOS/TOOLS/MemoryStaleWrite.test.ts new file mode 100644 index 0000000000..2351ebd5f5 --- /dev/null +++ b/LifeOS/install/LIFEOS/TOOLS/MemoryStaleWrite.test.ts @@ -0,0 +1,293 @@ +/** + * Stale-base write guard for the hot-layer memory files. + * + * The memory reviewer reads both files, spends ~2 minutes on inference, then + * REPLACES the file with its full list. Anything written in between was lost: + * a manual consolidation landed seconds before a review whose snapshot was + * minutes old, and the review undid it. The fix: the reviewer passes the + * fingerprint of the entries it read, and setEntries refuses (ESTALE_BASE) + * when the file's entries no longer match. + * + * MemoryWriter resolves the memory paths from homedir() at import time, and Bun + * reads HOME only at startup, so every scenario runs in its own `bun` process + * under a temp HOME. No live memory file is touched. + * + * Run: bun test LIFEOS/TOOLS/MemoryStaleWrite.test.ts + */ +import { describe, expect, test } from "bun:test"; +import { mkdirSync, mkdtempSync, readFileSync, symlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +const TOOLS = import.meta.dir; +const REL = "LIFEOS/USER/PRINCIPAL/PRINCIPAL_MEMORY.md"; + +function fixture(): { home: string; path: string } { + const home = mkdtempSync(join(tmpdir(), "memory-stale-write-")); + const claude = join(home, ".claude"); + // Mirror the real layout: LIFEOS/USER is a symlink into ~/.config/LIFEOS/USER, + // which MemorySystem's system/user boundary check requires. + const userData = join(home, ".config/LIFEOS/USER"); + for (const d of ["PRINCIPAL", "DIGITAL_ASSISTANT"]) mkdirSync(join(userData, d), { recursive: true }); + mkdirSync(join(claude, "LIFEOS/MEMORY/OBSERVABILITY"), { recursive: true }); + symlinkSync(userData, join(claude, "LIFEOS/USER")); + const entries = Array.from({ length: 12 }, (_, i) => `RULE: fixture fact ${i} ~explicit`); + const body = (list: string[]) => + `---\nschema_version: 1\nlast_updated: 2026-01-01T11:00:00.000Z\nlast_updated_by: fixture\n---\n\n# Memory\n\n\n${list.join("\n")}\n\n`; + const path = join(claude, REL); + writeFileSync(path, body(entries)); + writeFileSync(join(claude, "LIFEOS/USER/DIGITAL_ASSISTANT/DA_MEMORY.md"), body(["ROLE: fixture assistant fact ~explicit"])); + return { home, path }; +} + +/** + * Run `body` in a fresh process whose homedir() is the fixture home; the value + * it returns comes back through a result file. Not stdout: under `bun test` in + * this directory, Bun.spawnSync ran the child but captured an empty stdout. + */ +function scenario(home: string, body: string): any { + const id = Math.random().toString(36).slice(2); + const file = join(home, `scenario-${id}.ts`); + const result = join(home, `result-${id}.json`); + writeFileSync(file, ` + const fs = await import("node:fs"); + try { + const out = await (async () => { ${body} })(); + fs.writeFileSync(${JSON.stringify(result)}, JSON.stringify({ out: out ?? null })); + } catch (e) { + fs.writeFileSync(${JSON.stringify(result)}, JSON.stringify({ error: String(e?.stack ?? e) })); + } + `); + Bun.spawnSync(["bun", file], { cwd: home, env: { ...process.env, HOME: home } }); + let parsed: any; + try { + parsed = JSON.parse(readFileSync(result, "utf8")); + } catch { + throw new Error(`scenario produced no result file: ${file}`); + } + if (parsed.error) throw new Error(`scenario threw: ${parsed.error}`); + return parsed.out; +} + +/** Run a MemoryWriter scenario; `W` is the writer module and `P` the fixture principal file. */ +function run(home: string, code: string): any { + return scenario(home, ` + const W = await import(${JSON.stringify(join(TOOLS, "MemoryWriter.ts"))}); + const P = ${JSON.stringify(join(home, ".claude", REL))}; + ${code} + `); +} + +function writeLog(home: string): any[] { + const p = join(home, ".claude/LIFEOS/MEMORY/OBSERVABILITY/memory-writes.jsonl"); + try { + return readFileSync(p, "utf8").trim().split("\n").map((l) => JSON.parse(l)); + } catch { + return []; + } +} + +describe("stale-base guard", () => { + test("replay: a review write on a stale snapshot does not undo a manual consolidation", () => { + const { home } = fixture(); + const out = run(home, ` + const snapshot = W.read(P); // reviewer snapshot, before inference + const merged = snapshot.entries.slice(2).concat("RULE: fixture facts 0 and 1, merged ~explicit"); + const manual = W.setEntries(P, merged, { updatedBy: "manual-curation", allowDrastic: true }); // manual edit during inference + const review = W.setEntries(P, snapshot.entries.concat("RULE: new fact from review ~explicit"), + { updatedBy: "MemorySystem.add", expectedFingerprint: snapshot.fingerprint }); // review write after inference + return { manualOk: manual.ok, review, after: W.read(P).entries, merged }; + `); + expect(out.manualOk).toBe(true); + expect(out.review.ok).toBe(false); + expect(out.review.code).toBe("ESTALE_BASE"); + expect(out.after).toEqual(out.merged); + }); + + test("a matching fingerprint writes normally", () => { + const { home } = fixture(); + const out = run(home, ` + const snap = W.read(P); + return W.setEntries(P, snap.entries.concat("RULE: added ~explicit"), { expectedFingerprint: snap.fingerprint }); + `); + expect(out.ok).toBe(true); + expect(out.new_count).toBe(13); + }); + + test("a frontmatter-only change (timestamp) is not a stale base", () => { + const { home, path } = fixture(); + const out = run(home, ` + const snap = W.read(P); + const fs = await import("node:fs"); + fs.writeFileSync(P, fs.readFileSync(P, "utf8").replace(/last_updated: .*/, "last_updated: 2026-01-01T12:00:00.000Z")); + return W.setEntries(P, snap.entries.concat("RULE: added ~explicit"), { expectedFingerprint: snap.fingerprint }); + `); + expect(out.ok).toBe(true); + expect(readFileSync(path, "utf8")).toContain("RULE: added ~explicit"); + }); + + test("callers without a fingerprint behave as before", () => { + const { home } = fixture(); + const out = run(home, ` + const snap = W.read(P); + W.setEntries(P, snap.entries.concat("RULE: someone else ~explicit")); + return W.setEntries(P, snap.entries.concat("RULE: legacy caller ~explicit")); + `); + expect(out.ok).toBe(true); + }); + + test("a refusal is logged to memory-writes.jsonl with both fingerprints", () => { + const { home } = fixture(); + run(home, ` + const snap = W.read(P); + W.setEntries(P, snap.entries.concat("RULE: manual ~explicit"), { updatedBy: "manual" }); + return W.setEntries(P, snap.entries, { updatedBy: "MemorySystem.add", expectedFingerprint: snap.fingerprint }); + `); + const row = writeLog(home).find((r) => r.rejected === true); + expect(row?.rejection_code).toBe("ESTALE_BASE"); + expect(row?.updated_by).toBe("MemorySystem.add"); + expect(typeof row?.expected_fingerprint).toBe("string"); + expect(row?.expected_fingerprint).not.toBe(row?.actual_fingerprint); + }); +}); + +describe("lock handling", () => { + test("a lock released within the wait window is retried, not dropped", () => { + const { home } = fixture(); + // setEntries waits synchronously, so the lock is released by a child process. + const retried = run(home, ` + const fs = await import("node:fs"); + fs.writeFileSync(P + ".lock", ""); + Bun.spawn(["sh", "-c", "sleep 0.3; rm -f '" + P + ".lock'"]); + const snap = W.read(P); + return W.setEntries(P, snap.entries.concat("RULE: after lock ~explicit"), { expectedFingerprint: snap.fingerprint }); + `); + expect(retried.ok).toBe(true); + }); + + test("a lock that never releases fails with ELOCK_HELD and is logged", () => { + const { home } = fixture(); + const out = run(home, ` + const fs = await import("node:fs"); + fs.writeFileSync(P + ".lock", ""); + const t0 = Date.now(); + const r = W.setEntries(P, W.read(P).entries.concat("RULE: blocked ~explicit"), { updatedBy: "MemorySystem.add" }); + return { code: r.code, waitedMs: Date.now() - t0 }; + `); + expect(out.code).toBe("ELOCK_HELD"); + expect(out.waitedMs).toBeGreaterThanOrEqual(1500); + expect(out.waitedMs).toBeLessThan(5000); + expect(writeLog(home).some((r) => r.rejection_code === "ELOCK_HELD")).toBe(true); + }); +}); + +describe("health check", () => { + /** Run MemoryHealthCheck against the fixture root and return its report's finding ids. */ + function healthFindings(home: string, nowIso?: string): string[] { + const root = join(home, ".claude"); + Bun.spawnSync(["bun", join(TOOLS, "MemoryHealthCheck.ts")], { + cwd: home, + env: { ...process.env, HOME: home, CORTEX_HEALTH_ROOT: root, ...(nowIso ? { CORTEX_HEALTH_NOW: nowIso } : {}) }, + }); + const rows = readFileSync(join(root, "LIFEOS/MEMORY/OBSERVABILITY/memory-health.jsonl"), "utf8").trim().split("\n"); + return JSON.parse(rows[rows.length - 1]).findings.map((f: any) => f.id); + } + + test("a refused write warns for 24h, then clears", () => { + const { home } = fixture(); + run(home, ` + const snap = W.read(P); + W.setEntries(P, snap.entries.concat("RULE: manual ~explicit"), { updatedBy: "manual" }); + return W.setEntries(P, snap.entries, { updatedBy: "MemorySystem.add", expectedFingerprint: snap.fingerprint }); + `); + expect(healthFindings(home)).toContain("refused-writes"); + const later = new Date(Date.now() + 25 * 60 * 60 * 1000).toISOString(); + expect(healthFindings(home, later)).not.toContain("refused-writes"); + const earlier = new Date(Date.now() - 60 * 60 * 1000).toISOString(); + expect(healthFindings(home, earlier)).not.toContain("refused-writes"); // future-dated row not counted + }); + + test("a write dropped on a held lock also warns", () => { + const { home } = fixture(); + run(home, ` + const fs = await import("node:fs"); + fs.writeFileSync(P + ".lock", ""); + return W.setEntries(P, W.read(P).entries.concat("RULE: blocked ~explicit"), { updatedBy: "MemorySystem.add" }).code; + `); + expect(healthFindings(home)).toContain("refused-writes"); + }); +}); + +describe("reviewer wiring", () => { + test("dispatchItems refuses a memory set whose snapshot went stale, and the manual write survives", () => { + const { home } = fixture(); + const out = run(home, ` + const R = await import(${JSON.stringify(join(TOOLS, "MemoryReviewer.ts"))}); + const snap = R.readCurrentMemorySnapshot(); + W.setEntries(P, snap.principal.concat("RULE: manual fact ~explicit"), { updatedBy: "manual" }); + const { summary, results } = R.dispatchItems( + [{ type: "memory", actor: "principal", op: "set", entries: snap.principal.concat("RULE: review fact ~explicit") }], + { baseFingerprints: snap.fingerprints }, + ); + return { summary, results, after: W.read(P).entries }; + `); + // A refusal is the guard working: counted as a guard skip, not a failed run. + expect(out.summary.failed).toBe(0); + expect(out.summary.skipped_guard).toBe(1); + expect(JSON.stringify(out.results[0])).toContain("ESTALE_BASE"); + expect(out.after).toContain("RULE: manual fact ~explicit"); + expect(out.after).not.toContain("RULE: review fact ~explicit"); + }); + + test("with no concurrent edit, sets for both actors land (fingerprints wired to the right files)", () => { + const { home } = fixture(); + const out = run(home, ` + const R = await import(${JSON.stringify(join(TOOLS, "MemoryReviewer.ts"))}); + const snap = R.readCurrentMemorySnapshot(); + const { summary } = R.dispatchItems([ + { type: "memory", actor: "principal", op: "set", entries: snap.principal.concat("RULE: principal new ~explicit") }, + { type: "memory", actor: "assistant", op: "set", entries: snap.assistant.concat("ROLE: assistant new ~explicit") }, + ], { baseFingerprints: snap.fingerprints }); + const A = ${JSON.stringify(join(home, ".claude/LIFEOS/USER/DIGITAL_ASSISTANT/DA_MEMORY.md"))}; + return { summary, principal: W.read(P).entries, assistant: W.read(A).entries }; + `); + expect(out.summary.succeeded).toBe(2); + expect(out.principal).toContain("RULE: principal new ~explicit"); + expect(out.assistant).toContain("ROLE: assistant new ~explicit"); + }); + + test("a second item for the same actor builds on the first write, not the old snapshot", () => { + const { home } = fixture(); + const out = run(home, ` + const R = await import(${JSON.stringify(join(TOOLS, "MemoryReviewer.ts"))}); + const snap = R.readCurrentMemorySnapshot(); + const { summary } = R.dispatchItems([ + { type: "memory", actor: "principal", op: "set", entries: snap.principal.concat("RULE: first ~explicit") }, + { type: "memory", actor: "principal", op: "add", content: "RULE: second ~explicit" }, + ], { baseFingerprints: snap.fingerprints }); + return { summary, after: W.read(P).entries }; + `); + expect(out.summary.succeeded).toBe(2); + expect(out.after).toContain("RULE: first ~explicit"); + expect(out.after).toContain("RULE: second ~explicit"); + }); + + test("review() end to end with mocked inference passes its snapshot's fingerprints and writes", () => { + const { home } = fixture(); + const transcript = join(home, "transcript.jsonl"); + writeFileSync(transcript, [ + JSON.stringify({ timestamp: "2026-01-01T11:00:00Z", message: { role: "user", content: "remember that I prefer tea" } }), + JSON.stringify({ timestamp: "2026-01-01T11:00:05Z", message: { role: "assistant", content: "Noted." } }), + ].join("\n") + "\n"); + const fixtureEntries = Array.from({ length: 12 }, (_, i) => `RULE: fixture fact ${i} ~explicit`); + const mock = JSON.stringify({ items: [{ type: "memory", actor: "principal", op: "set", entries: fixtureEntries.concat("PREFERENCE: prefers tea ~explicit") }] }); + const out = run(home, ` + const R = await import(${JSON.stringify(join(TOOLS, "MemoryReviewer.ts"))}); + const res = await R.review({ input: ${JSON.stringify(transcript)}, mockInferenceResponse: ${JSON.stringify(mock)} }); + return { ok: res.ok, summary: res.dispatch_summary, after: W.read(P).entries }; + `); + expect(out.ok).toBe(true); + expect(out.summary.succeeded).toBe(1); + expect(out.after).toContain("PREFERENCE: prefers tea ~explicit"); + }); +}); diff --git a/LifeOS/install/LIFEOS/TOOLS/MemorySystem.ts b/LifeOS/install/LIFEOS/TOOLS/MemorySystem.ts index b57c7aa33c..becfae5768 100755 --- a/LifeOS/install/LIFEOS/TOOLS/MemorySystem.ts +++ b/LifeOS/install/LIFEOS/TOOLS/MemorySystem.ts @@ -403,7 +403,7 @@ function generateProposalId(): string { * subprocess) is responsible for trimming. add() surfaces the at-cap error * verbatim so the caller can re-submit with explicit eviction choices. */ -function addMemoryItem(item: TypedItem & { type: "memory" }, path: string): AddResult { +function addMemoryItem(item: TypedItem & { type: "memory" }, path: string, opts: AddOptions): AddResult { const current = memoryWriterRead(path); if ("code" in current) { return { ok: false, code: "EINVAL_ITEM", message: `Memory file unreadable: ${current.message}` }; @@ -431,7 +431,11 @@ function addMemoryItem(item: TypedItem & { type: "memory" }, path: string): AddR newEntries = [...current.entries, item.content.trim()]; } - const writeResult = memoryWriterSetEntries(path, newEntries, { updatedBy: "MemorySystem.add" }); + // Write only if the file still holds what the list was computed from: the + // caller's read for op:"set" (the reviewer's snapshot, minutes old), this + // function's own read for op:"add". + const expectedFingerprint = item.op === "set" ? opts.expectedFingerprint : current.fingerprint; + const writeResult = memoryWriterSetEntries(path, newEntries, { updatedBy: "MemorySystem.add", expectedFingerprint }); if (!writeResult.ok) { return { ok: false, @@ -451,6 +455,7 @@ function addMemoryItem(item: TypedItem & { type: "memory" }, path: string): AddR dropped_malformed: writeResult.dropped_malformed, dropped_overlength: writeResult.dropped_overlength, dropped_duplicates: writeResult.dropped_duplicates, + fingerprint: writeResult.fingerprint, }, }; } @@ -807,7 +812,16 @@ export function sanitizeTypedItemForPersistence(item: TypedItem): SanitizedItemR * item's resolved storage path's mutation tier matches the type's declared * tier — a defense-in-depth check against registry/classifier drift. */ -export function add(item: TypedItem): AddResult { +/** + * Orchestrator-side write context — deliberately NOT part of the item, whose + * shape is model-facing (the sanitizer rejects unknown item fields). + */ +export interface AddOptions { + /** For memory op:"set": fingerprint of the entries the list was computed from (MemoryWriter.read().fingerprint). */ + expectedFingerprint?: string; +} + +export function add(item: TypedItem, opts: AddOptions = {}): AddResult { if (!item || typeof item !== "object" || !("type" in item)) { return { ok: false, code: "EINVAL_ITEM", message: "Item missing 'type' field" }; } @@ -863,7 +877,7 @@ export function add(item: TypedItem): AddResult { switch (entry.write_mode) { case "set-overwrite": - return addMemoryItem(item as TypedItem & { type: "memory" }, path); + return addMemoryItem(item as TypedItem & { type: "memory" }, path, opts); case "append": return addNoteTypeItem(item as TypedItem & { type: "idea" | "knowledge" }, path); case "queue": { diff --git a/LifeOS/install/LIFEOS/TOOLS/MemoryWriter.ts b/LifeOS/install/LIFEOS/TOOLS/MemoryWriter.ts index 5122376711..52bc8a0cd5 100755 --- a/LifeOS/install/LIFEOS/TOOLS/MemoryWriter.ts +++ b/LifeOS/install/LIFEOS/TOOLS/MemoryWriter.ts @@ -14,7 +14,8 @@ * 5. Writes atomically: acquire .lock → write .tmp → atomic rename * * Why set-overwrite beats incremental add/replace/remove: - * - No race surface (single atomic write per review) + * - One atomic write per review (the list is computed from a read made minutes + * earlier, so callers pass expectedFingerprint to refuse a stale base) * - Idempotent (same input produces same file) * - Eviction is structural (model omits entries it wants gone) * - Simpler mental model: "here is the state I want" @@ -49,6 +50,7 @@ import { unlinkSync, writeFileSync, } from "node:fs"; +import { createHash } from "node:crypto"; import { dirname, resolve as pathResolve } from "node:path"; import { homedir } from "node:os"; @@ -85,6 +87,8 @@ export interface SetEntriesOk { new_count: number; evictions: string[]; additions: string[]; + /** Fingerprint of the entries just written — the base for a follow-up write. */ + fingerprint: string; } export interface SetEntriesErrAtCap { @@ -130,8 +134,21 @@ export interface SetEntriesErrErosion { new_count: number; } +/** + * The caller's view of the file is stale: its entries changed after the caller + * read them (see SetEntriesOptions.expectedFingerprint). Nothing was written. + */ +export interface SetEntriesErrStale { + ok: false; + code: "ESTALE_BASE"; + message: string; + expected_fingerprint: string; + actual_fingerprint: string; +} + export type SetEntriesResult = | SetEntriesOk + | SetEntriesErrStale | SetEntriesErrAtCap | SetEntriesErrPath | SetEntriesErrLock @@ -151,6 +168,8 @@ export interface ReadResult { * `entries`, so anything listed here is erased by its next write. */ dropped_invalid: { entry: string; reason: "malformed" | "overlength" }[]; + /** entriesFingerprint(entries) — pass back as expectedFingerprint to write only if nothing changed since. */ + fingerprint: string; } // ── Path validation ── @@ -235,6 +254,15 @@ function validateAndDedup(entries: string[]): ValidationOutcome { return { accepted, malformed, overlength, duplicates }; } +/** + * Fingerprint of an entry list: what a caller saw when it read the file. Entries + * only, never the raw bytes — frontmatter (last_updated) changes on every write, + * and a timestamp-only difference must not count as a changed file. + */ +export function entriesFingerprint(entries: string[]): string { + return createHash("sha256").update(entries.join("\n")).digest("hex"); +} + // ── File parse / serialize ── // Line-based canonical model (public PR #1593, @anikinsasha). The former @@ -350,24 +378,40 @@ export function serializeMemoryContent( // ── Atomic write with lock ── +// A write holds the lock for milliseconds, so a held lock is usually about to be +// released. Wait briefly rather than drop a write that may carry minutes of +// reviewer inference. +const LOCK_WAIT_MS = 2000; +const LOCK_POLL_MS = 50; + +function sleepSync(ms: number): void { + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms); +} + function withLock(filePath: string, action: () => T): T | SetEntriesErrLock | SetEntriesErrIO { const lockPath = `${filePath}.lock`; let fd: number | null = null; - try { - fd = openSync(lockPath, "wx"); // O_CREAT | O_EXCL - } catch (e: any) { - if (e?.code === "EEXIST") { - return { - ok: false, - code: "ELOCK_HELD", - message: `Lock held by another writer: ${lockPath}. Investigate stale lock if persistent.`, - }; + const deadline = Date.now() + LOCK_WAIT_MS; + while (fd === null) { + try { + fd = openSync(lockPath, "wx"); // O_CREAT | O_EXCL + } catch (e: any) { + if (e?.code !== "EEXIST") { + return { + ok: false, + code: "EWRITE_FAILED", + message: `Failed to acquire lock: ${e?.message || String(e)}`, + }; + } + if (Date.now() >= deadline) { + return { + ok: false, + code: "ELOCK_HELD", + message: `Lock held by another writer for ${LOCK_WAIT_MS}ms: ${lockPath}. Investigate stale lock if persistent.`, + }; + } + sleepSync(LOCK_POLL_MS); } - return { - ok: false, - code: "EWRITE_FAILED", - message: `Failed to acquire lock: ${e?.message || String(e)}`, - }; } try { @@ -494,9 +538,10 @@ function logWriteEvent( */ function logRejectedWrite( filePath: string, - rejection: { code: string; message: string; prior_count: number; new_count: number }, + rejection: { code: string; message: string; prior_count?: number; new_count?: number }, delta: { evictions: string[]; additions: string[] }, updatedBy?: string, + extra: Record = {}, ): void { appendWriteLog({ ts: new Date().toISOString(), @@ -509,6 +554,7 @@ function logRejectedWrite( new_count: rejection.new_count, evictions: delta.evictions, additions: delta.additions, + ...extra, }); } @@ -519,6 +565,13 @@ export interface SetEntriesOptions { updatedBy?: string; /** Bypass the catastrophic-shrink guard (legitimate full-clear / restore). */ allowDrastic?: boolean; + /** + * The `fingerprint` from the read this write was computed from. When given, + * the write is refused with ESTALE_BASE if the file's entries changed since — + * a set-overwrite built on an old read would otherwise silently undo every + * write in between (the reviewer's read→inference→write window is minutes). + */ + expectedFingerprint?: string; } export function setEntries( @@ -566,6 +619,33 @@ export function setEntries( const evictions = priorEntries.filter((e) => !newSet.has(e)); const additions = newEntries.filter((e) => !priorSet.has(e)); + // Stale-base guard, computed IN-LOCK: no lock-respecting writer can land + // between this check and the write (direct file writes bypass the lock). + // Compared against the same valid-entries view read() returns. + if (options.expectedFingerprint !== undefined) { + const actual = entriesFingerprint(validateAndDedup(priorEntries).accepted); + if (actual !== options.expectedFingerprint) { + const staleErr: SetEntriesErrStale = { + ok: false, + code: "ESTALE_BASE", + message: "Refused: the file's entries changed after this write's base was read. Nothing was written; re-read and recompute.", + expected_fingerprint: options.expectedFingerprint, + actual_fingerprint: actual, + }; + logRejectedWrite( + abs, + { code: staleErr.code, message: staleErr.message, prior_count: priorEntries.length, new_count: newEntries.length }, + // Not { evictions, additions }: measured against the CURRENT file they + // would list the newer edit's changes as the review's, and replaying them + // would undo that edit. The review's own intent is in its run-debug folder. + { evictions: [], additions: [] }, + options.updatedBy, + { expected_fingerprint: staleErr.expected_fingerprint, actual_fingerprint: actual }, + ); + return staleErr; + } + } + // Catastrophic-shrink guard (computed IN-LOCK against the just-read prior // state, so it can't race a concurrent write). set-overwrite REPLACES the // file, so a hallucinated empty/tiny reviewer list would wipe real memory @@ -629,11 +709,17 @@ export function setEntries( new_count: newEntries.length, evictions, additions, + fingerprint: entriesFingerprint(newEntries), }; logWriteEvent(abs, ok, options.updatedBy); return ok; }); + // A lock that never cleared dropped this write; log it, since the reviewer's + // stdio is ignored and a returned error alone is never seen. + if (!result.ok && result.code === "ELOCK_HELD") { + logRejectedWrite(abs, result, { evictions: [], additions: [] }, options.updatedBy); + } return result; } @@ -651,6 +737,7 @@ export function read(filePath: string): ReadResult | SetEntriesErrPath { cap_entries: MAX_ENTRIES, cap_chars: MAX_ENTRIES * MAX_CHARS_PER_ENTRY, dropped_invalid: [], + fingerprint: entriesFingerprint([]), }; } @@ -678,6 +765,7 @@ export function read(filePath: string): ReadResult | SetEntriesErrPath { cap_entries: MAX_ENTRIES, cap_chars: MAX_ENTRIES * MAX_CHARS_PER_ENTRY, dropped_invalid, + fingerprint: entriesFingerprint(valid.accepted), }; }