fix(close): a read-only brain writes no clean-shutdown evidence — the marker is the writer's word about itself

src/brainy.ts

@@ -20486,9 +20486,22 @@ export class Brainy<T = any> implements BrainyInterface<T> {
           await this._aggregationIndex.flush()
         }
       })(),
-      // 8.0 MVCC: detach the generation-bump hook and persist the counter
+      // 8.0 MVCC: detach the generation-bump hook and persist the counter.
+      // READ-ONLY GUARD: a reader's open() never sets the bump hook, never
+      // buffers pending single-ops, and — since generationStore.open() also
+      // leaves the clean-shutdown marker untouched for a reader — never
+      // consumes it either, so there is nothing of a writer's to persist or
+      // release here. Calling close() anyway would still WRITE: it
+      // unconditionally re-stamps `_system/clean-shutdown.json` (and can
+      // advance the fold checkpoint / counter files) at the generation this
+      // session merely observed — a reader vouching for a commit it never
+      // made. The marker is the writer's own evidence about the writer's own
+      // process; a read-only brain must leave `_system/` exactly as it found
+      // it. (Mirrors the same guard already applied to every other Phase-1
+      // step below, and to the signal-path shutdown in
+      // registerShutdownHooks().)
       (async () => {
-        if (this.generationStore) {
+        if (this.generationStore && !this.isReadOnly) {
           await this.generationStore.close()
         }
       })()

src/db/generationStore.ts

@@ -805,7 +805,16 @@ export class GenerationStore {
         if (uncleanOpen) await this.advanceFoldCheckpointUnlocked()
         // The marker is consumed: any session that can write invalidates it
         // at first commit (see the commit paths); a clean close re-writes it.
-        await this.clearCleanShutdownMarker()
+        // A READER NEVER CONSUMES IT. The marker is the writer's own evidence
+        // about the writer's own process — clearing it here exists so that
+        // if THIS session goes on to write and then dies before its next
+        // clean close, the marker's absence correctly reads as unclean. A
+        // reader can never write, so it can never leave the store in a state
+        // its own crash would mis-describe; clearing the marker for it would
+        // only cost the store's actual writer a needless whole-log fold on
+        // its next open, for a generation the reader merely observed. Leave
+        // `_system/` exactly as found.
+        if (!options?.readOnly) await this.clearCleanShutdownMarker()
       }
       await this.factLog.open(this.committed)
     } else {
@@ -895,7 +904,11 @@ export class GenerationStore {
     }
   }
 
-  /** Consume the clean-shutdown marker (every open; a clean close re-writes it). */
+  /**
+   * Consume the clean-shutdown marker (every WRITER open; a clean close
+   * re-writes it). Callers must gate this on `!options.readOnly` — a reader
+   * never consumes the marker, see the call site in {@link open}.
+   */
   private async clearCleanShutdownMarker(): Promise<void> {
     try {
       await this.storage.deleteRawObject(CLEAN_SHUTDOWN_PATH)

tests/integration/readonly-close-no-marker.test.ts

@@ -0,0 +1,250 @@
+/**
+ * @module tests/integration/readonly-close-no-marker
+ * @description A READ-ONLY BRAIN WRITES NO CLEAN-SHUTDOWN EVIDENCE.
+ *
+ * `_system/clean-shutdown.json` is the WRITER's own word about the writer's
+ * own process: "everything above this line, from THIS session, is durable."
+ * Two call sites treated a reader exactly like a writer:
+ *
+ *   1. `Brainy#closeDurableSteps()` called `generationStore.close()`
+ *      unconditionally — a reader's close re-stamped the marker at the
+ *      generation the reader merely OBSERVED, never committed.
+ *   2. `GenerationStore#open()` consumed (deleted) the marker on every open,
+ *      reader or writer alike, so a reader that never got to a matching
+ *      close left the store looking crashed to the next writer.
+ *
+ * Both are fixed by making a read-only brain leave `_system/` exactly as it
+ * found it — at open AND at close. Pinned here:
+ *
+ *   1. `_system/` is byte-for-byte identical (file set + contents) before and
+ *      after a reader opens a cleanly-closed store, reads it, and closes.
+ *   2. After the reader's close, the next WRITER open adopts the marker as
+ *      clean — no recovery fold narrates.
+ *   3. A reader creates no file under `_system/` merely by opening (before it
+ *      ever closes).
+ *   4. A reader that opens and is then abandoned (crash-style, no close) does
+ *      not force the next writer to pay a recovery fold — the concrete harm
+ *      the fix closes.
+ */
+
+import { describe, it, expect, beforeEach, afterEach } from 'vitest'
+import { mkdtempSync, rmSync, readdirSync, readFileSync, statSync } from 'node:fs'
+import { createHash } from 'node:crypto'
+import { tmpdir } from 'node:os'
+import { join } from 'node:path'
+import { Brainy } from '../../src/brainy.js'
+import { NounType } from '../../src/types/graphTypes.js'
+import { abandonAsCrashed } from '../helpers/durabilityKillMatrix.js'
+
+function makeTempDir(): string {
+  return mkdtempSync(join(tmpdir(), 'brainy-readonly-close-'))
+}
+
+/** Recursively hash every regular file under `dir`, keyed by its path relative to `dir`. */
+function snapshotDir(dir: string): Map<string, string> {
+  const out = new Map<string, string>()
+  const walk = (rel: string): void => {
+    const abs = rel ? join(dir, rel) : dir
+    let entries: string[]
+    try {
+      entries = readdirSync(abs)
+    } catch {
+      return
+    }
+    for (const name of entries) {
+      const childRel = rel ? join(rel, name) : name
+      const childAbs = join(dir, childRel)
+      const st = statSync(childAbs)
+      if (st.isDirectory()) {
+        walk(childRel)
+      } else if (st.isFile()) {
+        const hash = createHash('sha256').update(readFileSync(childAbs)).digest('hex')
+        out.set(childRel, hash)
+      }
+    }
+  }
+  walk('')
+  return out
+}
+
+/** Capture console.warn lines (the narration channel — see `prodLog.narrate`) while `fn` runs. */
+async function captureWarn<T>(fn: () => Promise<T>): Promise<{ result: T; lines: string[] }> {
+  const lines: string[] = []
+  const orig = console.warn
+  console.warn = ((...args: unknown[]) => {
+    lines.push(args.map((a) => String(a)).join(' '))
+  }) as typeof console.warn
+  try {
+    return { result: await fn(), lines }
+  } finally {
+    console.warn = orig
+  }
+}
+
+describe('a read-only brain writes no clean-shutdown evidence', () => {
+  let dir: string
+  let brain: Brainy | null = null
+
+  beforeEach(() => {
+    dir = makeTempDir()
+  })
+
+  afterEach(async () => {
+    if (brain) {
+      try {
+        await brain.close()
+      } catch {
+        /* already closed */
+      }
+      brain = null
+    }
+    try {
+      rmSync(dir, { recursive: true, force: true })
+    } catch {
+      /* ignore */
+    }
+  })
+
+  const systemDir = () => join(dir, '_system')
+  /**
+   * The marker file's actual on-disk name — `clean-shutdown.json` or, under
+   * FileSystemStorage's default gzip compression, `clean-shutdown.json.gz`.
+   * Returns null when absent.
+   */
+  const findMarkerPath = (): string | null => {
+    let entries: string[]
+    try {
+      entries = readdirSync(systemDir())
+    } catch {
+      return null
+    }
+    const name = entries.find((n) => n.startsWith('clean-shutdown.json'))
+    return name ? join(systemDir(), name) : null
+  }
+
+  it('leaves `_system/`\'s file set and the clean-shutdown marker\'s bytes identical across a reader open → read → close', async () => {
+    // A writer opens, writes, and closes cleanly — the marker lands at
+    // whatever generation the writer actually committed.
+    const writer = new Brainy({ requireSubtype: false, storage: { type: 'filesystem', path: dir } })
+    await writer.init()
+    await writer.add({ data: 'seed entity', type: NounType.Concept })
+    await writer.add({ data: 'second entity', type: NounType.Concept })
+    await writer.flush()
+    await writer.close()
+
+    const markerBeforePath = findMarkerPath()
+    expect(markerBeforePath, 'the writer left a clean-shutdown marker').not.toBeNull()
+    const before = snapshotDir(systemDir())
+    expect(before.size).toBeGreaterThan(0)
+    const markerBeforeHash = before.get(
+      (markerBeforePath as string).slice(systemDir().length + 1)
+    )
+    expect(markerBeforeHash).toBeTruthy()
+
+    // A reader opens the same store, reads, and closes.
+    brain = await Brainy.openReadOnly({ storage: { type: 'filesystem', path: dir } })
+    expect(brain.isReadOnly).toBe(true)
+    await brain.stats()
+    await brain.close()
+    brain = null
+
+    // The FILE SET under `_system/` is unchanged — a reader creates and
+    // removes nothing. (Other files under `_system/` — e.g. the metadata
+    // field registry, which stamps its own `lastUpdated` on every persist —
+    // are a pre-existing, separate concern outside this fix's scope: this
+    // pin is specifically about the generation store's clean-shutdown
+    // evidence, not about every subsystem's close() being a true no-op for
+    // a reader.)
+    const after = snapshotDir(systemDir())
+    expect([...after.keys()].sort()).toEqual([...before.keys()].sort())
+
+    // The MARKER's bytes are byte-for-byte identical — the reader neither
+    // consumed it at open nor re-stamped it at close.
+    const markerAfterPath = findMarkerPath()
+    expect(markerAfterPath, 'the marker must still exist, under the same name').toBe(markerBeforePath)
+    const markerAfterHash = after.get((markerAfterPath as string).slice(systemDir().length + 1))
+    expect(markerAfterHash).toBe(markerBeforeHash)
+  }, 120_000)
+
+  it('creates no file under `_system/` merely by opening read-only', async () => {
+    const writer = new Brainy({ requireSubtype: false, storage: { type: 'filesystem', path: dir } })
+    await writer.init()
+    await writer.add({ data: 'seed entity', type: NounType.Concept })
+    await writer.flush()
+    await writer.close()
+
+    const baselineNames = [...snapshotDir(systemDir()).keys()].sort()
+    expect(baselineNames.length).toBeGreaterThan(0)
+
+    // Open the reader and inspect `_system/` BEFORE it ever closes — open()
+    // alone must create nothing.
+    brain = await Brainy.openReadOnly({ storage: { type: 'filesystem', path: dir } })
+    const whileOpenNames = [...snapshotDir(systemDir()).keys()].sort()
+    expect(whileOpenNames).toEqual(baselineNames)
+
+    await brain.close()
+    brain = null
+  }, 120_000)
+
+  it('a writer reopening after the reader closes adopts the marker — no recovery fold', async () => {
+    const writer1 = new Brainy({ requireSubtype: false, storage: { type: 'filesystem', path: dir } })
+    await writer1.init()
+    await writer1.add({ data: 'seed entity', type: NounType.Concept })
+    await writer1.flush()
+    await writer1.close()
+
+    // A reader opens and closes in between — must not disturb the marker.
+    const reader = await Brainy.openReadOnly({ storage: { type: 'filesystem', path: dir } })
+    await reader.stats()
+    await reader.close()
+
+    // The next writer open must be a clean, no-fold open: no
+    // "log-authority recovery" / "WHOLE-LOG fold" narration line.
+    const { result: writer2, lines } = await captureWarn(async () => {
+      const w = new Brainy({ requireSubtype: false, storage: { type: 'filesystem', path: dir } })
+      await w.init()
+      return w
+    })
+    brain = writer2
+
+    const foldLines = lines.filter((l) => /log-authority recovery|WHOLE-LOG fold|recovery fold/i.test(l))
+    expect(foldLines, `unexpected recovery narration:\n${foldLines.join('\n')}`).toEqual([])
+
+    // And the store is exactly what the first writer left — the seed row is
+    // still there, nothing was rolled back or re-derived.
+    const found = await writer2.find({ where: {} } as any)
+    expect(found.length).toBeGreaterThanOrEqual(1)
+  }, 120_000)
+
+  it('a reader that opens and is then abandoned (never closes) does not force the next writer to fold', async () => {
+    // This is the concrete harm the fix closes: pre-fix, a reader's open()
+    // unconditionally DELETED the marker (consuming it as if it were the
+    // writer). A reader that opened and then died — no close, exactly like
+    // a killed process — left the marker gone, so the actual writer's next
+    // open read the store as crashed and paid a full recovery fold for a
+    // "crash" that was really just a reader that came and went.
+    const writer1 = new Brainy({ requireSubtype: false, storage: { type: 'filesystem', path: dir } })
+    await writer1.init()
+    await writer1.add({ data: 'seed entity', type: NounType.Concept })
+    await writer1.flush()
+    await writer1.close()
+
+    const reader = await Brainy.openReadOnly({ storage: { type: 'filesystem', path: dir } })
+    await reader.stats()
+    // NEVER calls reader.close() — abandon it exactly like a killed process.
+    await abandonAsCrashed(reader)
+
+    const { result: writer2, lines } = await captureWarn(async () => {
+      const w = new Brainy({ requireSubtype: false, storage: { type: 'filesystem', path: dir } })
+      await w.init()
+      return w
+    })
+    brain = writer2
+
+    const foldLines = lines.filter((l) => /log-authority recovery|WHOLE-LOG fold|recovery fold/i.test(l))
+    expect(
+      foldLines,
+      `an abandoned READER forced a recovery fold on the next writer open:\n${foldLines.join('\n')}`
+    ).toEqual([])
+  }, 120_000)
+})