diff --git a/src/brainy.ts b/src/brainy.ts index 39b604ad..02f2ca3d 100644 --- a/src/brainy.ts +++ b/src/brainy.ts @@ -20486,9 +20486,22 @@ export class Brainy implements BrainyInterface { 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() } })() diff --git a/src/db/generationStore.ts b/src/db/generationStore.ts index 128f905b..89f83a8f 100644 --- a/src/db/generationStore.ts +++ b/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 { try { await this.storage.deleteRawObject(CLEAN_SHUTDOWN_PATH) diff --git a/tests/integration/readonly-close-no-marker.test.ts b/tests/integration/readonly-close-no-marker.test.ts new file mode 100644 index 00000000..7bcf99df --- /dev/null +++ b/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 { + const out = new Map() + 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(fn: () => Promise): 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) +})