From fe42a7c087563d578d082b84b3c064df6fda9b68 Mon Sep 17 00:00:00 2001 From: alex anikin <60673011+anikinsasha@users.noreply.github.com> Date: Thu, 23 Jul 2026 19:27:14 -0700 Subject: [PATCH] fix(memory): canonical marker rebuild in MemoryWriter + one shared lenient parser for all readers The BEGIN/END ENTRIES marker handling had three compounding defects: 1. parseFile located END via first-indexOf over the whole body, so a stray END before BEGIN sent every write into the recovery branch forever. 2. serializeFile's self-heal appended a missing BEGIN at the END of the body (after any stray END), manufacturing a permanent END-before-BEGIN inversion, and re-emitted one fresh END per write: files grew a stack of duplicate END markers, +1 per curation write, never compacted. 3. Every read-side parser (LoadMemory hook, Pulse memory module, MemoryHealthCheck, MemoryRestore) used strict first-indexOf/lazy-regex logic that bails to zero entries on marker disorder - memory silently stopped loading into sessions while the writer kept curating, and the health check reported the corrupted file as healthy headroom (0/48). Files scaffolded by pre-7.0.0 templates (which shipped without the marker pair) are corrupted this way in the wild; install/copyMissing never re-templates an existing file, so they never recover on their own. Fix: - MemoryWriter parse is now line-based and uniformly lenient: markers count only as whole trimmed lines (an entry mentioning a marker can no longer truncate the block); entries = every valid-prefix line anywhere after frontmatter; invalid-prefix orphans stay in body verbatim; CRLF tolerated. - serialize always emits the canonical shape (frontmatter, body, BEGIN, entries, END, trailing newline): one write converges any corrupted file, repeated writes are byte-identical. - parseMemoryContent is exported and every reader imports it - reader and writer can never diverge again. - MemoryHealthCheck gains CHECK 7.5: marker structural sanity goes CRITICAL on corruption instead of false-green. - validateAndDedup rejects submitted entries containing marker substrings (counted as dropped_malformed). - The inline smoke test no longer touches the LIVE principal memory file (it previously wiped it down to its fixture entries on a populated install); it now runs against .memtest.md fixtures confined to the OS temp dir, and covers the corrupted shapes: END-before-BEGIN + END stack, marker-substring entries, CRLF, frontmatter-less, invalid-prefix orphans, idempotency. --- LifeOS/install/LIFEOS/PULSE/modules/memory.ts | 11 +- .../install/LIFEOS/TOOLS/MemoryHealthCheck.ts | 61 ++- LifeOS/install/LIFEOS/TOOLS/MemoryRestore.ts | 6 +- .../install/LIFEOS/TOOLS/MemoryWriter.test.ts | 264 +++++++++++++ LifeOS/install/LIFEOS/TOOLS/MemoryWriter.ts | 350 +++++++++++++----- LifeOS/install/hooks/LoadMemory.hook.ts | 20 +- 6 files changed, 602 insertions(+), 110 deletions(-) create mode 100644 LifeOS/install/LIFEOS/TOOLS/MemoryWriter.test.ts diff --git a/LifeOS/install/LIFEOS/PULSE/modules/memory.ts b/LifeOS/install/LIFEOS/PULSE/modules/memory.ts index 22db4510aa..e69d43b856 100644 --- a/LifeOS/install/LIFEOS/PULSE/modules/memory.ts +++ b/LifeOS/install/LIFEOS/PULSE/modules/memory.ts @@ -28,6 +28,7 @@ import { statSync, } from "node:fs"; import { join } from "node:path"; +import { parseMemoryContent } from "../../TOOLS/MemoryWriter"; const HOME = process.env.HOME || ""; const CLAUDE = join(HOME, ".claude"); @@ -109,13 +110,9 @@ function safeReadJsonLines(path: string, lastN: number = 50): any[] { function readMemoryFile(path: string): { entries: string[]; count: number; charsUsed: number } { if (!existsSync(path)) return { entries: [], count: 0, charsUsed: 0 }; try { - const raw = readFileSync(path, "utf-8"); - const start = raw.indexOf(""); - const end = raw.indexOf(""); - if (start === -1 || end === -1 || end < start) return { entries: [], count: 0, charsUsed: 0 }; - const block = raw.slice(start + "".length, end).trim(); - if (!block) return { entries: [], count: 0, charsUsed: 0 }; - const entries = block.split("\n").map((l) => l.trim()).filter((l) => l.length > 0); + // Shared lenient parser (MemoryWriter owns it) — the old strict indexOf + // slice showed an empty panel on marker-corrupted files. + const { entries } = parseMemoryContent(readFileSync(path, "utf-8")); const charsUsed = entries.reduce((s, e) => s + e.length, 0); return { entries, count: entries.length, charsUsed }; } catch { diff --git a/LifeOS/install/LIFEOS/TOOLS/MemoryHealthCheck.ts b/LifeOS/install/LIFEOS/TOOLS/MemoryHealthCheck.ts index 9dfe303b51..265e774b0f 100755 --- a/LifeOS/install/LIFEOS/TOOLS/MemoryHealthCheck.ts +++ b/LifeOS/install/LIFEOS/TOOLS/MemoryHealthCheck.ts @@ -23,6 +23,13 @@ import { existsSync, readFileSync, appendFileSync, mkdirSync, readdirSync, statSync } from "node:fs"; import { join } from "node:path"; +import { parseMemoryContent, read as memoryRead, BEGIN_MARKER, END_MARKER } from "./MemoryWriter"; + +// CLI script, not a library: the checks below run at module top level and end +// in process.exit. Spawn it (MemoryHealthGate does); never import it. +if (!import.meta.main) { + throw new Error("MemoryHealthCheck.ts is a CLI script with top-level side effects — spawn it via bun, never import it."); +} const HOME = process.env.HOME || ""; const CLAUDE = join(HOME, ".claude"); @@ -246,13 +253,63 @@ else add("principal-memory-present", "ok", "PRINCIPAL_MEMORY.md present."); if (!existsSync(DA_MEM)) add("da-memory-missing", "critical", "DA_MEMORY.md missing."); else add("da-memory-present", "ok", "DA_MEMORY.md present."); +// CHECK 7.5: marker structural sanity — exactly one BEGIN and one END, in order. +// The 2026-07 corruption (END-before-BEGIN + a duplicate-END stack growing +1 +// per write) blinded every strict reader to 0 entries while cap-pressure kept +// reporting green headroom. Structural damage must be a red signal; the next +// canonical MemoryWriter write heals it. +for (const [label, path] of [["principal", PRINCIPAL_MEM], ["da", DA_MEM]] as const) { + if (!existsSync(path)) continue; + try { + const lines = readFileSync(path, "utf-8").split("\n").map(l => l.trim()); + const begins = lines.filter(l => l === BEGIN_MARKER).length; + const ends = lines.filter(l => l === END_MARKER).length; + const ordered = lines.indexOf(BEGIN_MARKER) !== -1 && lines.indexOf(BEGIN_MARKER) < lines.indexOf(END_MARKER); + if (begins !== 1 || ends !== 1 || !ordered) { + add(`marker-corrupt:${label}`, "critical", + `${label} memory marker structure corrupt (${begins} BEGIN / ${ends} END${ordered ? "" : ", disordered"}) — awaiting canonical-write heal.`, + { begins, ends }); + } else { + add(`marker-ok:${label}`, "ok", `${label} memory marker pair canonical.`); + } + } catch (e) { + // A health probe must record its own failure, never kill the run and + // leave the previous healthy row standing. + add(`marker-check-error:${label}`, "critical", `${label} marker-sanity check failed to read the file: ${e instanceof Error ? e.message : String(e)}`); + } +} + +// CHECK 7.6: pending silent loss — on-disk entries the writer-side read() +// excludes as invalid (over-length / marker-content / bad prefix). The +// reviewer's set-overwrite submits read() output, so anything reported here +// is erased by its next curation write; nag until the entry is repaired. +for (const [label, path] of [["principal", PRINCIPAL_MEM], ["da", DA_MEM]] as const) { + if (!existsSync(path)) continue; + let r: ReturnType; + try { + r = memoryRead(path); + } catch (e) { + add(`pending-loss-check-error:${label}`, "critical", `${label} pending-loss check failed to read the file: ${e instanceof Error ? e.message : String(e)}`); + continue; + } + if (!("dropped_invalid" in r)) continue; + if (r.dropped_invalid.length > 0) { + add(`pending-silent-loss:${label}`, "warn", + `${label} memory: ${r.dropped_invalid.length} on-disk entr${r.dropped_invalid.length === 1 ? "y" : "ies"} invalid to read() — the next curation write erases them: ${r.dropped_invalid.map(d => `[${d.reason}] ${d.entry.slice(0, 60)}…`).join(" · ")}`, + { count: r.dropped_invalid.length }); + } else { + add(`no-pending-loss:${label}`, "ok", `${label} memory: read() accepts all on-disk entries.`); + } +} + // CHECK 8: cap-pressure — the exact failure class that sat silent for two weeks. // A file AT cap can't accept new memory; near-cap means the next curation must // consolidate or it jams. (Eviction now works, so this is a warning not a freeze.) function entryCount(path: string): number { if (!existsSync(path)) return 0; - const m = readFileSync(path, "utf-8").match(/([\s\S]*?)/); - return m ? m[1].split("\n").map(l => l.trim()).filter(l => l.length > 0).length : 0; + // Shared lenient parser — the old lazy regex matched the first (empty) + // BEGIN→END pair on corrupted files and reported 0/48 headroom, green. + return parseMemoryContent(readFileSync(path, "utf-8")).entries.length; } for (const [label, path] of [["principal", PRINCIPAL_MEM], ["da", DA_MEM]] as const) { const n = entryCount(path); diff --git a/LifeOS/install/LIFEOS/TOOLS/MemoryRestore.ts b/LifeOS/install/LIFEOS/TOOLS/MemoryRestore.ts index 8d7cbdbc94..ab2183c9ed 100644 --- a/LifeOS/install/LIFEOS/TOOLS/MemoryRestore.ts +++ b/LifeOS/install/LIFEOS/TOOLS/MemoryRestore.ts @@ -15,6 +15,7 @@ import { readdirSync, readFileSync, writeFileSync, existsSync } from "node:fs"; import { resolve as pathResolve } from "node:path"; import { homedir } from "node:os"; +import { parseMemoryContent } from "./MemoryWriter"; const CLAUDE_ROOT = pathResolve(homedir(), ".claude"); const SNAPSHOT_DIR = pathResolve(CLAUDE_ROOT, "LIFEOS/MEMORY/OBSERVABILITY/memory-snapshots"); @@ -32,9 +33,8 @@ function snapshotsFor(which: string): string[] { } function countEntries(content: string): number { - const m = content.match(/([\s\S]*?)/); - if (!m) return 0; - return m[1].split("\n").map((l) => l.trim()).filter((l) => l.length > 0).length; + // Shared lenient parser — the old lazy regex read 0 on marker-corrupted files. + return parseMemoryContent(content).entries.length; } function main(): void { diff --git a/LifeOS/install/LIFEOS/TOOLS/MemoryWriter.test.ts b/LifeOS/install/LIFEOS/TOOLS/MemoryWriter.test.ts new file mode 100644 index 0000000000..65409db6a7 --- /dev/null +++ b/LifeOS/install/LIFEOS/TOOLS/MemoryWriter.test.ts @@ -0,0 +1,264 @@ +/** + * MemoryWriter.test.ts — corruption-shape fixtures for the canonical-rebuild + * parser/serializer (task 359, 2026-07-23 marker-corruption fix). + * + * Write-path tests use temp fixtures via the constrained .memtest.md escape + * (OS temp dir only) — the LIVE memory files are never touched. + */ + +import { describe, expect, test } from "bun:test"; +import { mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { + BEGIN_MARKER, + END_MARKER, + parseMemoryContent, + serializeMemoryContent, + setEntries, + read, +} from "./MemoryWriter"; + +const stripStamp = (s: string) => s.replace(/^last_updated: .*$/m, "last_updated: X"); +const markerCount = (s: string, m: string) => s.split("\n").filter((l) => l.trim() === m).length; + +const CORRUPTED = [ + "---", + "schema_version: 1", + "last_updated: 2026-07-01T00:00:00.000Z", + "---", + "# Hot-Layer Memory", + "", + "", + END_MARKER, + BEGIN_MARKER, + END_MARKER, + END_MARKER, + END_MARKER, + "FACT: legacy invalid-prefix orphan ~explicit", + "NAME: Fixture User", + "PREFERENCE: prefers fixtures over live files", + `RULE: keep the ${END_MARKER} marker pair intact`, + END_MARKER, +].join("\n") + "\n"; + +describe("parseMemoryContent — corrupted shapes", () => { + test("END-before-BEGIN + END stack: recovers all valid-prefix entries, keeps orphan in body", () => { + const p = parseMemoryContent(CORRUPTED); + expect(p.entries).toEqual([ + "NAME: Fixture User", + "PREFERENCE: prefers fixtures over live files", + `RULE: keep the ${END_MARKER} marker pair intact`, + ]); + expect(p.bodyLines.some((l) => l.trim().startsWith("FACT: "))).toBe(true); + expect(p.bodyLines.some((l) => l.trim() === BEGIN_MARKER || l.trim() === END_MARKER)).toBe(false); + }); + + test("entry containing a literal marker parses whole (no truncation)", () => { + const p = parseMemoryContent(CORRUPTED); + expect(p.entries[2]).toContain(END_MARKER); + }); + + test("CRLF file parses and canonicalizes with frontmatter intact", () => { + const crlf = "---\r\nschema_version: 1\r\n---\r\nNAME: Crlf User\r\n"; + const p = parseMemoryContent(crlf); + expect(p.entries).toEqual(["NAME: Crlf User"]); + const s = serializeMemoryContent(p, p.entries, "test"); + expect(s).toMatch(/^last_updated: /m); + expect(s).not.toContain("\r"); + }); + + test("frontmatter-less file parses; serialize is stable", () => { + const bare = `NAME: Bare User\n${END_MARKER}\n`; + const p = parseMemoryContent(bare); + expect(p.frontmatter).toBe(""); + expect(p.entries).toEqual(["NAME: Bare User"]); + const s1 = serializeMemoryContent(p, p.entries, "test"); + const p2 = parseMemoryContent(s1); + const s2 = serializeMemoryContent(p2, p2.entries, "test"); + expect(s1).toBe(s2); + }); + + test("frontmatter-only file yields empty entries and canonical empty block", () => { + const fmOnly = "---\nschema_version: 1\n---\n"; + const p = parseMemoryContent(fmOnly); + expect(p.entries).toEqual([]); + const s = serializeMemoryContent(p, [], "test"); + expect(markerCount(s, BEGIN_MARKER)).toBe(1); + expect(markerCount(s, END_MARKER)).toBe(1); + }); +}); + +describe("serializeMemoryContent — canonical rebuild", () => { + test("one write converges any corruption to exactly one ordered marker pair", () => { + const p = parseMemoryContent(CORRUPTED); + const s = serializeMemoryContent(p, p.entries, "test"); + expect(markerCount(s, BEGIN_MARKER)).toBe(1); + expect(markerCount(s, END_MARKER)).toBe(1); + const lines = s.split("\n").map((l) => l.trim()); + expect(lines.indexOf(BEGIN_MARKER)).toBeLessThan(lines.indexOf(END_MARKER)); + expect(s.endsWith("\n")).toBe(true); + }); + + test("idempotent: serialize∘parse∘serialize is byte-stable (modulo timestamp)", () => { + const p1 = parseMemoryContent(CORRUPTED); + const s1 = serializeMemoryContent(p1, p1.entries, "test"); + const p2 = parseMemoryContent(s1); + const s2 = serializeMemoryContent(p2, p2.entries, "test"); + expect(stripStamp(s2)).toBe(stripStamp(s1)); + }); + + test("zero entry loss through canonicalization (verbatim set-diff empty)", () => { + const before = parseMemoryContent(CORRUPTED).entries; + const after = parseMemoryContent( + serializeMemoryContent(parseMemoryContent(CORRUPTED), before, "test"), + ).entries; + expect(new Set(after)).toEqual(new Set(before)); + }); +}); + +describe("setEntries write path — temp fixtures only", () => { + const tmp = mkdtempSync(join(tmpdir(), "memwriter-test-")); + + test("heals a corrupted on-disk file in one write, stable in two, orphan untouched", () => { + const f = join(tmp, "heal.memtest.md"); + writeFileSync(f, CORRUPTED, "utf8"); + const entries = parseMemoryContent(readFileSync(f, "utf8")).entries; + + // Production path note: the reviewer submits read() output, which excludes + // the marker-substring entry BEFORE the write — the write-side + // dropped_malformed below only shows up because this test submits the raw + // parse. read().dropped_invalid is the signal production relies on. + const pre = read(f); + if ("dropped_invalid" in pre) { + expect(pre.dropped_invalid).toEqual([ + { entry: `RULE: keep the ${END_MARKER} marker pair intact`, reason: "malformed" }, + ]); + } + + const w1 = setEntries(f, entries, { updatedBy: "test" }); + expect(w1.ok).toBe(true); + if (w1.ok) { + expect(w1.accepted).toBe(2); + expect(w1.dropped_malformed).toBe(1); + } + const healed = readFileSync(f, "utf8"); + expect(markerCount(healed, BEGIN_MARKER)).toBe(1); + expect(markerCount(healed, END_MARKER)).toBe(1); + expect(healed).toContain("FACT: legacy invalid-prefix orphan"); + expect(healed).not.toContain(`RULE: keep the ${END_MARKER}`); + + const w2 = setEntries(f, parseMemoryContent(healed).entries, { updatedBy: "test" }); + expect(w2.ok).toBe(true); + expect(stripStamp(readFileSync(f, "utf8"))).toBe(stripStamp(healed)); + + const r = read(f); + expect("count" in r && r.count).toBe(2); + }); + + test("rejects a submitted entry containing marker text as malformed", () => { + const f = join(tmp, "marker-entry.memtest.md"); + writeFileSync(f, `---\nx: 1\n---\n${BEGIN_MARKER}\n${END_MARKER}\n`, "utf8"); + const w = setEntries(f, ["NAME: Ok", `RULE: sneaky ${BEGIN_MARKER} inside`], { updatedBy: "test" }); + expect(w.ok).toBe(true); + if (w.ok) { + expect(w.accepted).toBe(1); + expect(w.dropped_malformed).toBe(1); + } + }); + + test("catastrophic-shrink guard still trips on near-wipe of a populated file", () => { + const f = join(tmp, "shrink.memtest.md"); + const many = Array.from({ length: 12 }, (_, i) => `PREFERENCE: entry number ${i}`); + writeFileSync(f, `---\nx: 1\n---\n${BEGIN_MARKER}\n${many.join("\n")}\n${END_MARKER}\n`, "utf8"); + const w = setEntries(f, ["PREFERENCE: entry number 0"], { updatedBy: "test" }); + expect(w.ok).toBe(false); + if (!w.ok) expect(w.code).toBe("ESUSPECT_SHRINK"); + }); + + test("temp escape stays confined: vault-shaped path outside tmpdir is rejected", () => { + const w = setEntries("/Users/laptop/Desktop/evil.memtest.md", ["NAME: x"], { updatedBy: "test" }); + expect(w.ok).toBe(false); + if (!w.ok) expect(w.code).toBe("EINVAL_PATH"); + }); + + test("temp escape stays confined: tmpdir symlink pointing outside is rejected", () => { + const link = join(tmp, "sneaky.memtest.md"); + symlinkSync("/etc/hosts", link); + const w = setEntries(link, ["NAME: x"], { updatedBy: "test" }); + expect(w.ok).toBe(false); + if (!w.ok) expect(w.code).toBe("EINVAL_PATH"); + }); + + test("read() reports over-length on-disk entries as pending drops, not silently", () => { + const f = join(tmp, "overlength.memtest.md"); + const big = `PREFERENCE: ${"Y".repeat(300)}`; + writeFileSync(f, `---\nx: 1\n---\n${BEGIN_MARKER}\nNAME: Ok\n${big}\n${END_MARKER}\n`, "utf8"); + const r = read(f); + expect("dropped_invalid" in r).toBe(true); + if ("dropped_invalid" in r) { + expect(r.count).toBe(1); + expect(r.dropped_invalid).toEqual([{ entry: big, reason: "overlength" }]); + } + }); + + test("symlink at the .tmp write-target cannot be written through (O_EXCL)", () => { + const f = join(tmp, "wt.memtest.md"); + const victim = join(tmp, "victim-not-memtest.txt"); + writeFileSync(f, `---\nx: 1\n---\n${BEGIN_MARKER}\nNAME: Seed\n${END_MARKER}\n`, "utf8"); + writeFileSync(victim, "ORIGINAL", "utf8"); + symlinkSync(victim, `${f}.tmp`); + const w = setEntries(f, ["NAME: Seed", "RULE: written via the real file"], { updatedBy: "test" }); + // The write must still succeed (O_EXCL unlinks the stale symlink first) and + // the victim must be untouched — never written through the symlink. + expect(w.ok).toBe(true); + expect(readFileSync(victim, "utf8")).toBe("ORIGINAL"); + expect(readFileSync(f, "utf8")).toContain("RULE: written via the real file"); + }); + + test("embedded-newline entry is rejected, keeping one entry = one line", () => { + const f = join(tmp, "nl.memtest.md"); + writeFileSync(f, `---\nx: 1\n---\n${BEGIN_MARKER}\n${END_MARKER}\n`, "utf8"); + const w = setEntries(f, ["NAME: A\nRULE: smuggled second line"], { updatedBy: "test" }); + expect(w.ok).toBe(true); + if (w.ok) { + expect(w.accepted).toBe(0); + expect(w.dropped_malformed).toBe(1); + } + expect(read(f)).toMatchObject({ count: 0 }); + }); + + test("near-total wipe with one added entry still trips the shrink guard", () => { + const f = join(tmp, "wipe.memtest.md"); + const many = Array.from({ length: 20 }, (_, i) => `PREFERENCE: durable fact number ${i}`); + writeFileSync(f, `---\nx: 1\n---\n${BEGIN_MARKER}\n${many.join("\n")}\n${END_MARKER}\n`, "utf8"); + // Keep 2 + add 1 rephrased = 3 new entries (defeats the old floor<3 + additions>0 escape). + const w = setEntries(f, [many[0], many[1], "PREFERENCE: a rephrased consolidation"], { updatedBy: "test" }); + expect(w.ok).toBe(false); + if (!w.ok) expect(w.code).toBe("ESUSPECT_SHRINK"); + }); + + test("cleanup", () => { + rmSync(tmp, { recursive: true, force: true }); + expect(true).toBe(true); + }); +}); + +describe("parseMemoryContent — frontmatter mis-close does not swallow the entries block", () => { + test("unterminated frontmatter closing on a body --- still recovers entries", () => { + // Opening --- with no proper close; a later body '---' would let the greedy + // frontmatter regex swallow the marker block. Demotion recovers the entries. + const bad = [ + "---", + "schema_version: 1", + "", + BEGIN_MARKER, + "NAME: Should Be Found", + "RULE: also found", + END_MARKER, + "---", + ].join("\n") + "\n"; + const p = parseMemoryContent(bad); + expect(p.entries).toEqual(["NAME: Should Be Found", "RULE: also found"]); + }); +}); diff --git a/LifeOS/install/LIFEOS/TOOLS/MemoryWriter.ts b/LifeOS/install/LIFEOS/TOOLS/MemoryWriter.ts index 86de367d6a..b04bbbfd1e 100755 --- a/LifeOS/install/LIFEOS/TOOLS/MemoryWriter.ts +++ b/LifeOS/install/LIFEOS/TOOLS/MemoryWriter.ts @@ -49,8 +49,9 @@ import { unlinkSync, writeFileSync, } from "node:fs"; -import { dirname, resolve as pathResolve } from "node:path"; -import { homedir } from "node:os"; +import { dirname, join, resolve as pathResolve } from "node:path"; +import { homedir, tmpdir } from "node:os"; +import { mkdtempSync, realpathSync } from "node:fs"; // ── Constants ── @@ -65,8 +66,8 @@ const PREFIX_PATTERN = /^(NAME|ROLE|RELATION|PREFERENCE|RULE): /; const MAX_CHARS_PER_ENTRY = 256; const MAX_ENTRIES = 48; -const BEGIN_MARKER = ""; -const END_MARKER = ""; +export const BEGIN_MARKER = ""; +export const END_MARKER = ""; const OBSERVABILITY_PATH = pathResolve( CLAUDE_ROOT, @@ -136,6 +137,13 @@ export interface ReadResult { chars_used: number; cap_entries: number; cap_chars: number; + /** + * On-disk entries excluded from `entries` as invalid (bad/marker content or + * over-length). NEVER silently ignorable: the reviewer's set-overwrite + * submits `entries`, so anything listed here is erased by its next write. + * Health CHECK 7.6 surfaces these as pending silent loss until resolved. + */ + dropped_invalid: { entry: string; reason: "malformed" | "overlength" }[]; } // ── Path validation ── @@ -147,7 +155,7 @@ function validatePath(filePath: string): { ok: true; abs: string } | SetEntriesE } catch (e) { return { ok: false, code: "EINVAL_PATH", message: `Cannot resolve path: ${filePath}` }; } - if (!ALLOWED_FILES.has(abs)) { + if (!ALLOWED_FILES.has(abs) && !isTestPath(abs)) { return { ok: false, code: "EINVAL_PATH", @@ -157,6 +165,37 @@ function validatePath(filePath: string): { ok: true; abs: string } | SetEntriesE return { ok: true, abs }; } +// Test fixtures only: confined to the OS temp dir with a dedicated suffix, so +// the escape hatch can never target vault/user paths. This keeps the smoke +// test and bun tests off the LIVE memory files (the pre-2026-07-23 smoke test +// wrote to the real PRINCIPAL_MEMORY.md and its cleanup emptied it). +// Confinement is checked on REAL paths too, so a tmpdir symlink named +// *.memtest.md cannot point reads/snapshots at files outside the temp dir. +function isTestPath(abs: string): boolean { + let tmpRoot: string; + try { + tmpRoot = realpathSync(tmpdir()); + } catch { + tmpRoot = pathResolve(tmpdir()); + } + const lexOk = + abs.endsWith(".memtest.md") && + (abs.startsWith(pathResolve(tmpdir()) + "/") || abs.startsWith(tmpRoot + "/")); + if (!lexOk) return false; + try { + const real = realpathSync(abs); + return real.endsWith(".memtest.md") && real.startsWith(tmpRoot + "/"); + } catch { + // Not created yet: confine by the real parent directory instead. + try { + const realDir = realpathSync(dirname(abs)); + return realDir === tmpRoot || realDir.startsWith(tmpRoot + "/"); + } catch { + return false; + } + } +} + // ── Entry validation ── interface ValidationOutcome { @@ -177,12 +216,29 @@ function validateAndDedup(entries: string[]): ValidationOutcome { const entry = raw.trim(); if (entry.length === 0) continue; + // One entry = one physical line. An embedded newline would serialize as + // multiple on-disk lines, inflating the real entry count past every cap + // and desyncing accepted/new_count from what reparse sees. + if (/[\r\n]/.test(entry)) { + malformed++; + continue; + } + const m = entry.match(PREFIX_PATTERN); if (!m) { malformed++; continue; } + // Entries may never contain the structural markers: a marker substring + // inside an entry would blind naive parsers and pollute the block. A + // pre-existing on-disk offender still parses whole (line-based markers), + // but resubmission drops it here — visible as dropped_malformed. + if (entry.includes(BEGIN_MARKER) || entry.includes(END_MARKER)) { + malformed++; + continue; + } + // Length check: total entry length must be ≤ prefix.length + MAX_CHARS_PER_ENTRY // Equivalently: the content AFTER the prefix must be ≤ MAX_CHARS_PER_ENTRY. const prefixWithColonSpace = m[0]; // e.g. "PREFERENCE: " @@ -205,53 +261,63 @@ function validateAndDedup(entries: string[]): ValidationOutcome { // ── File parse / serialize ── -interface ParsedFile { +// Line-based canonical model. Markers are recognized ONLY as whole trimmed +// lines, so an entry that merely mentions a marker can never truncate the +// block. Parse is uniformly lenient: every valid-prefix line anywhere after +// the frontmatter is an entry (block ∪ orphans, order-preserved, first-seen +// deduped); marker lines are structural and dropped; everything else is body, +// preserved verbatim — including invalid-prefix orphans (e.g. a stray `FACT:` +// line), which are never silently absorbed or deleted. Serialize always emits +// the canonical shape (frontmatter → body → BEGIN → entries → END → newline), +// so a single write converges any historical corruption (END-before-BEGIN, +// duplicate-END stacks) and repeated writes are byte-identical. +// +// This is THE parser for the memory files. Every consumer (LoadMemory hook, +// Pulse memory panel, MemoryHealthCheck, MemoryRestore, Telegram context) +// imports it — never a second marker-parsing implementation. + +export interface ParsedMemoryFile { frontmatter: string; - preEntriesBody: string; + bodyLines: string[]; entries: string[]; - postEntriesBody: string; } -function parseFile(content: string): ParsedFile { - // Frontmatter is between two --- lines at the top. - const fmMatch = content.match(/^---\n([\s\S]*?)\n---\n/); - const frontmatter = fmMatch ? fmMatch[0] : ""; - const afterFm = content.slice(frontmatter.length); - - const beginIdx = afterFm.indexOf(BEGIN_MARKER); - const endIdx = afterFm.indexOf(END_MARKER); - - if (beginIdx === -1 || endIdx === -1 || endIdx < beginIdx) { - // Markers missing or malformed. Recover any orphaned prefixed entries a prior - // broken write may have dumped into the body (see serializeFile's BEGIN-marker - // heal) so read() still surfaces them and the next write re-homes them inside a - // proper marker block instead of duplicating them. Non-entry lines stay in - // preEntriesBody, preserving principal-authored content. - const recovered: string[] = []; - const kept: string[] = []; - for (const line of afterFm.split("\n")) { - const t = line.trim(); - if (t.length > 0 && PREFIX_PATTERN.test(t)) recovered.push(t); - else kept.push(line); +export function parseMemoryContent(content: string): ParsedMemoryFile { + const fmMatch = content.match(/^---\r?\n[\s\S]*?\r?\n---\r?\n/); + // A "frontmatter" that swallowed a marker line is a mis-close: an + // unterminated opening fence closing on some later body `---` would hide the + // whole entries block inside frontmatter (blinding the parse AND the shrink + // guard while marker-sanity checks stay green). Demote to no-frontmatter so + // every entry is recovered from the body instead. + const fmRaw = fmMatch ? fmMatch[0] : ""; + const fmValid = fmRaw !== "" && !fmRaw.includes(BEGIN_MARKER) && !fmRaw.includes(END_MARKER); + const frontmatter = fmValid ? fmRaw.replace(/\r\n/g, "\n") : ""; + const afterFm = fmValid ? content.slice(fmRaw.length) : content; + + const bodyLines: string[] = []; + const entries: string[] = []; + const seen = new Set(); + + for (const line of afterFm.split(/\r?\n/)) { + const t = line.trim(); + if (t === BEGIN_MARKER || t === END_MARKER) continue; + if (t.length > 0 && PREFIX_PATTERN.test(t)) { + if (!seen.has(t)) { + seen.add(t); + entries.push(t); + } + continue; } - return { - frontmatter, - preEntriesBody: kept.join("\n"), - entries: recovered, - postEntriesBody: "", - }; + bodyLines.push(line); } - const preEntriesBody = afterFm.slice(0, beginIdx + BEGIN_MARKER.length); - const entriesBlock = afterFm.slice(beginIdx + BEGIN_MARKER.length, endIdx); - const postEntriesBody = afterFm.slice(endIdx); - - const entries = entriesBlock - .split("\n") - .map((line) => line.trim()) - .filter((line) => line.length > 0 && PREFIX_PATTERN.test(line)); + // Trailing blank body lines are separator artifacts; serialize re-adds + // exactly one, keeping parse→serialize→parse byte-stable. + while (bodyLines.length > 0 && bodyLines[bodyLines.length - 1].trim() === "") { + bodyLines.pop(); + } - return { frontmatter, preEntriesBody, entries, postEntriesBody }; + return { frontmatter, bodyLines, entries }; } function updateFrontmatterTimestamp(frontmatter: string): string { @@ -273,32 +339,20 @@ function updateFrontmatterUpdatedBy(frontmatter: string, by: string): string { return frontmatter.replace(/\n---\n$/, `\nlast_updated_by: ${by}\n---\n`); } -function serializeFile(parsed: ParsedFile, newEntries: string[], updatedBy: string): string { +export function serializeMemoryContent( + parsed: ParsedMemoryFile, + newEntries: string[], + updatedBy: string, +): string { let fm = updateFrontmatterTimestamp(parsed.frontmatter); fm = updateFrontmatterUpdatedBy(fm, updatedBy); - // Ensure preEntriesBody ends just after BEGIN_MARKER, with a newline before entries. - // Symmetric self-heal (mirrors the END-marker heal below): if the body lost its - // BEGIN marker, re-insert it so appended entries land inside a parseable block. - // parseFile only recovers entries between BEGIN and END, so without this an - // already-broken file would keep accepting writes that never persist as entries. - let pre = parsed.preEntriesBody; - if (!pre.includes(BEGIN_MARKER)) { - if (pre.length > 0 && !pre.endsWith("\n")) pre += "\n"; - pre += BEGIN_MARKER; - } - if (!pre.endsWith("\n")) pre += "\n"; - - const entriesBlock = newEntries.length === 0 ? "" : newEntries.join("\n") + "\n"; - - let post = parsed.postEntriesBody; - // Ensure post starts cleanly with the END_MARKER - if (!post.startsWith(END_MARKER)) { - // Should not happen given parseFile guarantees, but defensive - post = END_MARKER + post; - } - - return fm + pre + entriesBlock + post; + let out = fm; + if (parsed.bodyLines.length > 0) out += parsed.bodyLines.join("\n") + "\n\n"; + out += BEGIN_MARKER + "\n"; + if (newEntries.length > 0) out += newEntries.join("\n") + "\n"; + out += END_MARKER + "\n"; + return out; } // ── Atomic write with lock ── @@ -371,7 +425,12 @@ function snapshotBeforeWrite(absPath: string, priorContent: string): void { function atomicWrite(filePath: string, content: string): true | SetEntriesErrIO { const tmpPath = `${filePath}.tmp`; try { - writeFileSync(tmpPath, content, "utf8"); + // O_EXCL ("wx") after a best-effort unlink: a symlink planted at the + // predictable tmpPath would otherwise be written THROUGH (the write hits + // its target before the rename ever runs — confirmed attack). O_EXCL + // refuses any pre-existing path, symlinks included. + try { unlinkSync(tmpPath); } catch { /* absent is the normal case */ } + writeFileSync(tmpPath, content, { encoding: "utf8", flag: "wx" }); // fsync the tmp file for durability before rename const fd = openSync(tmpPath, "r+"); try { @@ -380,6 +439,12 @@ function atomicWrite(filePath: string, content: string): true | SetEntriesErrIO closeSync(fd); } renameSync(tmpPath, filePath); + // fsync the containing directory so the rename itself is durable — without + // it a power/kernel crash can roll the rename back despite ok:true. + try { + const dirFd = openSync(dirname(filePath), "r"); + try { fsyncSync(dirFd); } finally { closeSync(dirFd); } + } catch { /* dir fsync unsupported on some FSes — best-effort */ } return true; } catch (e: any) { try { unlinkSync(tmpPath); } catch { /* ignore */ } @@ -462,7 +527,7 @@ export function setEntries( const result = withLock(abs, () => { const content = readFileSync(abs, "utf8"); - const parsed = parseFile(content); + const parsed = parseMemoryContent(content); const priorEntries = parsed.entries; const newEntries = validated.accepted; @@ -476,18 +541,22 @@ export function setEntries( // 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 - // (this exact wipe happened once during a cross-vendor audit). We block two - // shapes only — and deliberately ALLOW large honest consolidation (many - // drops accompanied by additions), so the reviewer can still shrink hard - // when it's genuinely merging. Bypass for legitimate full-clears via opts. + // (this exact wipe happened once during a cross-vendor audit). Guard on the + // RETENTION ratio (how much of prior survived), not on additions: a near- + // total wipe that also adds one token entry is still a wipe. A genuine + // consolidation keeps most facts (reworded entries still match by content + // and count as retained) so it retains a high fraction; only a real + // hallucinated-list wipe drops most of prior. Bypass legitimate hard + // clears/restores via opts.allowDrastic. if (!options.allowDrastic && priorEntries.length >= 10) { const FLOOR = 3; - const massDeleteNoAdd = evictions.length > priorEntries.length * 0.5 && additions.length === 0; - if (newEntries.length < FLOOR || massDeleteNoAdd) { + const retained = priorEntries.length - evictions.length; + const retentionRatio = retained / priorEntries.length; + if (newEntries.length < FLOOR || retentionRatio < 0.5) { const shrinkErr: SetEntriesErrShrink = { ok: false, code: "ESUSPECT_SHRINK", - message: `Refused: op would shrink ${priorEntries.length} → ${newEntries.length} entries (${evictions.length} dropped, ${additions.length} added). Near-empty results and mass-deletion-without-curation are blocked as likely-bad output. A real consolidation that drops many should also ADD merged entries.`, + message: `Refused: op drops ${evictions.length}/${priorEntries.length} prior entries (only ${retained} retained, ${(retentionRatio * 100).toFixed(0)}%; ${additions.length} added). A near-total replacement is blocked as likely-bad output regardless of additions — real curation keeps most facts. Pass allowDrastic for an intentional hard clear/restore.`, prior_count: priorEntries.length, new_count: newEntries.length, }; @@ -498,7 +567,7 @@ export function setEntries( // Snapshot the prior content before we overwrite — individual-write recovery. snapshotBeforeWrite(abs, content); - const newContent = serializeFile(parsed, newEntries, options.updatedBy || "MemoryWriter"); + const newContent = serializeMemoryContent(parsed, newEntries, options.updatedBy || "MemoryWriter"); const writeRes = atomicWrite(abs, newContent); if (writeRes !== true) return writeRes; @@ -533,13 +602,25 @@ export function read(filePath: string): ReadResult | SetEntriesErrPath { chars_used: 0, cap_entries: MAX_ENTRIES, cap_chars: MAX_ENTRIES * MAX_CHARS_PER_ENTRY, + dropped_invalid: [], }; } const content = readFileSync(abs, "utf8"); - const parsed = parseFile(content); - // Silent-drop malformed entries discovered at read time too. + const parsed = parseMemoryContent(content); + // Entries invalid at read time are excluded from `entries` but REPORTED, + // never silently swallowed: a set-overwrite computed from `entries` would + // otherwise erase them with no trace anywhere (the write's dropped_* counts + // only cover the submission, which by then no longer contains them). const valid = validateAndDedup(parsed.entries); + const acceptedSet = new Set(valid.accepted); + const dropped_invalid: ReadResult["dropped_invalid"] = []; + for (const entry of parsed.entries) { + if (acceptedSet.has(entry)) continue; + const m = entry.match(PREFIX_PATTERN); + const overlength = !!m && entry.slice(m[0].length).length > MAX_CHARS_PER_ENTRY; + dropped_invalid.push({ entry, reason: overlength ? "overlength" : "malformed" }); + } const chars_used = valid.accepted.reduce((sum, e) => sum + e.length, 0); return { @@ -548,14 +629,90 @@ export function read(filePath: string): ReadResult | SetEntriesErrPath { chars_used, cap_entries: MAX_ENTRIES, cap_chars: MAX_ENTRIES * MAX_CHARS_PER_ENTRY, + dropped_invalid, }; } // ── CLI ── +// Timestamp-insensitive comparison: serialize stamps last_updated per call. +function stripStamp(s: string): string { + return s.replace(/^last_updated: .*$/m, "last_updated: X"); +} + +function countMarkerLines(s: string, marker: string): number { + return s.split("\n").filter((l) => l.trim() === marker).length; +} + function smokeTest(): number { console.log("MemoryWriter smoke test starting…"); - const testFile = pathResolve(CLAUDE_ROOT, "LIFEOS/USER/PRINCIPAL/PRINCIPAL_MEMORY.md"); + + // ── Pure canonical-rebuild fixtures (no filesystem) ── + const corrupted = + [ + "---", + "schema_version: 1", + "---", + "# Hot-Layer Memory", + "", + "", + END_MARKER, + BEGIN_MARKER, + END_MARKER, + END_MARKER, + "FACT: legacy invalid-prefix orphan stays in body ~explicit", + "NAME: Fixture User", + `RULE: keep the ${END_MARKER} marker pair intact`, + END_MARKER, + ].join("\n") + "\n"; + + const p1 = parseMemoryContent(corrupted); + if (p1.entries.length !== 2) { + console.error(`FAIL: corrupted fixture expected 2 entries, got ${p1.entries.length}`); + return 1; + } + if (p1.entries[1] !== `RULE: keep the ${END_MARKER} marker pair intact`) { + console.error(`FAIL: marker-substring entry truncated on parse: ${p1.entries[1]}`); + return 1; + } + if (!p1.bodyLines.some((l) => l.startsWith("FACT: "))) { + console.error("FAIL: invalid-prefix orphan not preserved in body"); + return 1; + } + const s1 = serializeMemoryContent(p1, p1.entries, "smoke"); + const p2 = parseMemoryContent(s1); + const s2 = serializeMemoryContent(p2, p2.entries, "smoke"); + if (stripStamp(s1) !== stripStamp(s2)) { + console.error("FAIL: canonical rebuild not idempotent (s1 ≠ s2 modulo timestamp)"); + return 1; + } + if (countMarkerLines(s1, BEGIN_MARKER) !== 1 || countMarkerLines(s1, END_MARKER) !== 1) { + console.error("FAIL: serialized output does not contain exactly one marker pair"); + return 1; + } + const crlf = "---\r\nschema_version: 1\r\n---\r\nNAME: Crlf User\r\n"; + const pc = parseMemoryContent(crlf); + if (pc.entries.length !== 1 || pc.entries[0] !== "NAME: Crlf User") { + console.error("FAIL: CRLF fixture did not parse to 1 entry"); + return 1; + } + const sc = serializeMemoryContent(pc, pc.entries, "smoke"); + if (stripStamp(sc) !== stripStamp(serializeMemoryContent(parseMemoryContent(sc), parseMemoryContent(sc).entries, "smoke"))) { + console.error("FAIL: CRLF fixture not stable after canonicalization"); + return 1; + } + const bare = `NAME: Bare User\n${END_MARKER}\n`; + const pb = parseMemoryContent(bare); + if (pb.entries.length !== 1 || pb.frontmatter !== "") { + console.error("FAIL: frontmatter-less fixture misparsed"); + return 1; + } + console.log(" pure fixtures: corrupted/CRLF/frontmatter-less all canonicalize, idempotent"); + + // ── Filesystem legs against a temp fixture (never the live files) ── + const tmpDir = mkdtempSync(join(tmpdir(), "memwriter-smoke-")); + const testFile = join(tmpDir, "PRINCIPAL_MEMORY.memtest.md"); + writeFileSync(testFile, "---\nschema_version: 1\n---\n\n# Fixture\n\n" + BEGIN_MARKER + "\n" + END_MARKER + "\n", "utf8"); const writer = "smoke-test"; // 1. Read initial state (should be empty) @@ -573,6 +730,7 @@ function smokeTest(): number { "PREFERENCE: Smoke-test prefers terse outputs", "RULE: Smoke-test always cleans up after itself", "INVALID_PREFIX: this should be dropped", + `RULE: sneaks in a ${END_MARKER} marker`, // marker-substring: rejected at validate `PREFERENCE: ${longStr}`, "NAME: SmokeTest User", // duplicate ]; @@ -586,8 +744,8 @@ function smokeTest(): number { console.error(`FAIL: expected 3 accepted, got ${w1.accepted}`); return 1; } - if (w1.dropped_malformed !== 1) { - console.error(`FAIL: expected 1 malformed drop, got ${w1.dropped_malformed}`); + if (w1.dropped_malformed !== 2) { + console.error(`FAIL: expected 2 malformed drops (bad prefix + marker substring), got ${w1.dropped_malformed}`); return 1; } if (w1.dropped_overlength !== 1) { @@ -648,13 +806,39 @@ function smokeTest(): number { } console.log(` write 4 (/etc/passwd): correctly rejected with ${w4.code}`); - // 7. Cleanup — restore to empty + // 7. On-disk heal: corrupted fixture converges to canonical in one write, stable in two + const healFile = join(tmpDir, "HEAL.memtest.md"); + writeFileSync(healFile, corrupted, "utf8"); + const healParsed = parseMemoryContent(readFileSync(healFile, "utf8")); + const h1 = setEntries(healFile, healParsed.entries, { updatedBy: writer }); + if (!h1.ok) { + console.error(`FAIL: heal write returned error: ${(h1 as any).message}`); + return 1; + } + const healed1 = readFileSync(healFile, "utf8"); + if (countMarkerLines(healed1, BEGIN_MARKER) !== 1 || countMarkerLines(healed1, END_MARKER) !== 1) { + console.error("FAIL: healed file does not have exactly one marker pair"); + return 1; + } + if (!healed1.includes("FACT: legacy invalid-prefix orphan")) { + console.error("FAIL: heal dropped the invalid-prefix body orphan"); + return 1; + } + const h2 = setEntries(healFile, parseMemoryContent(healed1).entries, { updatedBy: writer }); + if (!h2.ok || stripStamp(readFileSync(healFile, "utf8")) !== stripStamp(healed1)) { + console.error("FAIL: second heal write not byte-stable (modulo timestamp)"); + return 1; + } + console.log(` heal: corrupted fixture → canonical in one write, stable in two, orphan preserved`); + + // 8. Cleanup — remove the temp fixtures (live files were never touched) const w5 = setEntries(testFile, [], { updatedBy: "smoke-test-cleanup" }); if (!w5.ok) { console.error(`FAIL: cleanup write returned error: ${(w5 as any).message}`); return 1; } console.log(` cleanup: ${w5.new_count} entries remaining (should be 0)`); + try { rmSync(tmpDir, { recursive: true, force: true }); } catch { /* best-effort */ } console.log("✓ MemoryWriter smoke test PASSED"); return 0; diff --git a/LifeOS/install/hooks/LoadMemory.hook.ts b/LifeOS/install/hooks/LoadMemory.hook.ts index 0dcdf9b605..63c94a0115 100755 --- a/LifeOS/install/hooks/LoadMemory.hook.ts +++ b/LifeOS/install/hooks/LoadMemory.hook.ts @@ -23,14 +23,12 @@ import { existsSync, readFileSync } from "node:fs"; import { resolve as pathResolve } from "node:path"; import { homedir } from "node:os"; +import { parseMemoryContent } from "../LIFEOS/TOOLS/MemoryWriter"; const CLAUDE_ROOT = pathResolve(homedir(), ".claude"); const PRINCIPAL_MEMORY = pathResolve(CLAUDE_ROOT, "LIFEOS/USER/PRINCIPAL/PRINCIPAL_MEMORY.md"); const DA_MEMORY = pathResolve(CLAUDE_ROOT, "LIFEOS/USER/DIGITAL_ASSISTANT/DA_MEMORY.md"); -const ENTRIES_START = ""; -const ENTRIES_END = ""; - interface MemoryRead { entries: string[]; count: number; @@ -48,18 +46,10 @@ function isSubagentInvocation(): boolean { function readMemory(path: string): MemoryRead { if (!existsSync(path)) return { entries: [], count: 0, charsUsed: 0 }; try { - const raw = readFileSync(path, "utf8"); - const startIdx = raw.indexOf(ENTRIES_START); - const endIdx = raw.indexOf(ENTRIES_END); - if (startIdx === -1 || endIdx === -1 || endIdx < startIdx) { - return { entries: [], count: 0, charsUsed: 0 }; - } - const block = raw.slice(startIdx + ENTRIES_START.length, endIdx).trim(); - if (!block) return { entries: [], count: 0, charsUsed: 0 }; - const entries = block - .split("\n") - .map((l) => l.trim()) - .filter((l) => l.length > 0); + // Shared lenient parser (MemoryWriter owns it) — never a second local + // marker-parsing implementation. The old strict indexOf slice here went + // silently blind (0 entries) on marker-corrupted files for weeks. + const { entries } = parseMemoryContent(readFileSync(path, "utf8")); const charsUsed = entries.reduce((sum, e) => sum + e.length, 0); return { entries, count: entries.length, charsUsed }; } catch {