diff --git a/docs/architecture/multiprocess-storage-mixin.md b/docs/architecture/multiprocess-storage-mixin.md new file mode 100644 index 00000000..e975d865 --- /dev/null +++ b/docs/architecture/multiprocess-storage-mixin.md @@ -0,0 +1,135 @@ +# Design note: multi-process storage mixin + +**Status:** Proposed (future minor) +**Owner:** Brainy core +**Filed:** 2026-05-15 +**Companion:** [`concepts/storage-adapters`](../concepts/storage-adapters.md) + +## Context + +Brainy 7.21 added seven storage-adapter methods to support multi-process +safety: + +``` +supportsMultiProcessLocking() +acquireWriterLock(opts) +releaseWriterLock() +readWriterLock() +startFlushRequestWatcher(cb) +stopFlushRequestWatcher() +requestFlushOverFilesystem(timeoutMs) +``` + +They live on `BaseStorage` as no-op defaults and are overridden on +`FileSystemStorage` with real implementations. Any adapter extending +`FileSystemStorage` (e.g. Cortex's `MmapFileSystemStorage`) inherits the +real ones for free. + +This works correctly today. The question is whether the methods *belong* +on `BaseStorage`. + +## The case for moving them out + +`BaseStorage` already mixes several concerns: +- entity / verb CRUD primitives +- COW (copy-on-write) lifecycle +- type-statistics tracking +- count persistence +- multi-process safety (new) + +Adapters that have no notion of multi-process semantics — `MemoryStorage`, +cloud adapters (S3, GCS, R2, Azure, OPFS) — still carry seven inherited +no-ops on their prototype chain. A reader can't tell from the class +declaration whether a given adapter participates in the locking protocol; +it has to call `supportsMultiProcessLocking()` and trust the answer. + +A cleaner separation: + +```typescript +interface MultiProcessSafeStorage { + supportsMultiProcessLocking(): boolean + acquireWriterLock(opts?: { force?: boolean }): Promise + releaseWriterLock(): Promise + readWriterLock(): Promise + startFlushRequestWatcher(cb: () => Promise): void + stopFlushRequestWatcher(): void + requestFlushOverFilesystem(timeoutMs: number): Promise +} + +function isMultiProcessSafe(s: BaseStorage): s is BaseStorage & MultiProcessSafeStorage { + return typeof (s as any).supportsMultiProcessLocking === 'function' + && (s as any).supportsMultiProcessLocking() +} +``` + +Brainy's call sites become: + +```typescript +if (this.config.mode !== 'reader' && isMultiProcessSafe(this.storage)) { + await this.storage.acquireWriterLock({ force: this.config.force }) + // ... TypeScript narrows the rest correctly ... +} +``` + +Benefits: +- Type system enforces the capability — no more `(this.storage as any).X()`. +- Adapters that opt out (memory, cloud) are visibly clean. +- `hasStorageMethod()` defensive helper can stay (still guards + build/install artifacts) but doesn't carry the conceptual weight of + "did the plugin implement the interface." +- ADR-style trail for future capability additions: each new capability + gets its own interface, opted into explicitly. + +## The case against doing it now + +- Breaking change for any adapter that already overrides these methods. + `FileSystemStorage` is the only one in-tree, but Cortex's + `MmapFileSystemStorage` inherits from it — interface relocation would + ripple through the plugin ecosystem. +- The current state works. Real failure modes + (BR-CX-INTERFACE-GAP) were build/install artifacts, not type-system + failures. +- 7.22.0 just shipped a clean fix. Stacking another refactor before + consumers absorb it adds churn without urgency. +- The `hasStorageMethod()` guard accomplishes the same runtime safety the + interface narrowing would in TypeScript-aware code. + +## Recommendation + +**Defer.** Keep the current architecture through the 7.x line. Revisit +when: +- A second multi-process capability lands (e.g. distributed-readers + coordination) and the natural surface area is more than seven + methods. Five+ becomes the moment a separate interface earns its + keep. +- A v8 major is on the table for unrelated reasons. Bundle the + interface extraction with that release so consumers absorb both + changes in one upgrade. + +Until then: +- Document the inheritance contract (done — see + [`concepts/storage-adapters`](../concepts/storage-adapters.md)). +- Keep `hasStorageMethod()` as the runtime guard. +- Don't add new methods to `BaseStorage` defaults without re-evaluating + the surface-area boundary. + +## Migration sketch (when we do it) + +For reference, a clean migration path: + +1. Add `MultiProcessSafeStorage` interface to `src/storage/coreTypes.ts`. +2. Move the seven method signatures from `BaseStorage` to the new + interface. Default implementations stay on `BaseStorage` but only as + private helpers consumed by `FileSystemStorage`'s explicit + implementations. +3. `FileSystemStorage implements MultiProcessSafeStorage` becomes + explicit; methods get the `public` modifier with full JSDoc. +4. Brainy call sites switch from `hasStorageMethod` to + `isMultiProcessSafe` type-guard. Keep `hasStorageMethod` for + build/install artifact protection. +5. Document the new contract in `concepts/storage-adapters.md`. +6. Major-version-bump the `@soulcraft/brainy` peerDep range expected by + plugins. + +Estimated work: ~half a day of code, ~2 hours of doc/example updates, +ecosystem coordination via the platform handoff. diff --git a/docs/concepts/storage-adapters.md b/docs/concepts/storage-adapters.md new file mode 100644 index 00000000..a8da693b --- /dev/null +++ b/docs/concepts/storage-adapters.md @@ -0,0 +1,189 @@ +--- +title: Storage Adapter Inheritance Contract +slug: concepts/storage-adapters +public: true +category: concepts +template: concept +order: 6 +description: How storage adapters extend Brainy's BaseStorage and FileSystemStorage to inherit multi-process safety, what plugin authors need to override, and the contract Brainy promises to keep stable. +next: + - concepts/multi-process + - guides/inspection +--- + +# Storage Adapter Inheritance Contract + +Brainy's storage layer is designed for plugins to extend cleanly. A plugin +that subclasses `BaseStorage` or `FileSystemStorage` inherits new behavior +Brainy adds over time without code changes — provided the import resolution +brings in the right Brainy version at runtime. + +This page documents what plugin authors can rely on, what they need to +override, and the install-time failure modes that the defensive +`hasStorageMethod()` guard exists to handle. + +## The class hierarchy + +``` +BaseStorageAdapter (counts, batch ops, multi-tenancy hooks) + ↑ +BaseStorage (COW, type-statistics, lifecycle helpers, + default no-op multi-process methods) + ↑ +FileSystemStorage (real filesystem I/O, writer-lock implementation, + flush-request watcher, atomic writes) + ↑ + (Cortex's MmapFileSystemStorage, etc.) +``` + +When you `extend FileSystemStorage`, your adapter inherits every method on +the chain — including the ones Brainy adds in a later release — for free. +JavaScript's prototype chain resolves method lookups dynamically; nothing +about the inheritance is baked in at class-definition time. + +## What you inherit for free + +A plugin whose storage class extends `FileSystemStorage` automatically gets +the full multi-process safety surface: + +| Method | What it does | Override? | +|---|---|---| +| `acquireWriterLock(opts)` | Write `_writer.lock`, start heartbeat, throw on conflict | No | +| `releaseWriterLock()` | Clean up lock file + heartbeat timer | No | +| `readWriterLock()` | Read lock file as `WriterLockInfo` | No | +| `startFlushRequestWatcher(cb)` | Poll `_flush_requests/` and invoke `cb` | No | +| `stopFlushRequestWatcher()` | Stop the polling timer | No | +| `requestFlushOverFilesystem(timeoutMs)` | Drop a `.req`, await `.ack` | No | +| `supportsMultiProcessLocking()` | Return `false` (default) | **Yes — override to `true`** | + +The only required override is the capability flag. Returning `true` from +`supportsMultiProcessLocking()` is the signal Brainy uses to decide whether +to call `acquireWriterLock()` at init. + +```typescript +import { FileSystemStorage } from '@soulcraft/brainy' + +export class MmapFileSystemStorage extends FileSystemStorage { + public supportsMultiProcessLocking(): boolean { + return true + } + // ... your mmap-specific overrides ... +} +``` + +That's the full ceremony for inheriting multi-process safety. + +## When NOT to extend FileSystemStorage + +If your storage is **not filesystem-backed** (S3, GCS, R2, Azure, a custom +network backend), extend `BaseStorage` directly: + +```typescript +import { BaseStorage } from '@soulcraft/brainy' + +export class MyCloudStorage extends BaseStorage { + // BaseStorage's default no-op implementations of the multi-process + // methods stay in effect. `supportsMultiProcessLocking()` returns false + // by default — keep it that way unless you've implemented an object- + // versioned lease or similar cross-process synchronization for your + // backend. +} +``` + +Brainy treats cloud backends as not-multi-process-safe by default and logs a +one-line warning at init. That's the correct behavior until cloud locking +ships (currently out of scope — see +[`concepts/multi-process`](./multi-process.md)). + +## What `hasStorageMethod()` actually guards against + +The defensive check at every new-storage-method call site (`brainy.ts`, +`hasStorageMethod(name)`) does **not** exist to handle "plugin bundles a +stale BaseStorage." Plugins ship a dist that preserves the dynamic ESM +import (verify in your plugin's `dist/`: `import { FileSystemStorage } from +'@soulcraft/brainy'` is not rewritten to a vendored copy). The prototype +chain at runtime resolves to whatever Brainy version your consumer has +installed. + +`hasStorageMethod()` protects against **build/install artifacts** that break +the prototype chain at the consumer-app level: + +- **Stale `node_modules`** — a lingering install from before the consumer + upgraded Brainy. The package.json says `@soulcraft/brainy@7.22.0` but + `node_modules/@soulcraft/brainy` is still 7.20.x. +- **Lockfile drift** — `bun.lockb` / `package-lock.json` pins a brainy + version older than the package.json range, and `bun install` honors the + lockfile. +- **Docker layer cache** — the image reuses a `node_modules` from an + earlier build that predates the brainy bump. +- **Bundler quirks** — some bundlers (esbuild, webpack) flatten the + prototype chain at build time and lose later prototype mutations. Brainy + doesn't mutate prototypes at runtime, but bundler behavior can still + cause method lookups to fail in non-Node environments. + +In any of those, calling `storage.acquireWriterLock(...)` unconditionally +throws `TypeError: storage.acquireWriterLock is not a function`. The guard +turns that into a logged warning + graceful no-op so the app still boots, +and the warning names the adapter class plus a remediation hint: + +``` +[brainy] Storage adapter `MmapFileSystemStorage` is missing the multi-process +methods on its prototype chain. Writer locking and the flush-request RPC are +disabled for this directory. Likely fix: clean install (`rm -rf node_modules +bun.lockb && bun install`) or rebuild your container image to refresh +`@soulcraft/brainy` to ≥7.21. See docs/concepts/storage-adapters.md. +``` + +## Authoring a new storage adapter — minimum checklist + +1. **Extend the right base class.** + - Filesystem-backed → `FileSystemStorage`. + - Cloud / network / custom → `BaseStorage`. + +2. **Override the capability flag.** If filesystem-backed: + ```typescript + public supportsMultiProcessLocking(): boolean { return true } + ``` + +3. **Don't `super.X()`-wrap the multi-process methods**. They're inherited; + leaving them inherited means `hasStorageMethod()` finds them on the + prototype chain. Re-declaring them as `super.X()` wrappers makes the + helper resolve to your wrapper, which can fool the guard if your + constructor runs before the super class initializes. + +4. **Do override `init()` / `flush()` / `close()`** as needed. Always call + `super.init()` / `super.flush()` / `super.close()` first so the + filesystem prep, writer-lock acquisition, and lock release happen in the + expected order. + +5. **Verify the inheritance.** A one-line smoke test in your plugin's + `__tests__/`: + ```typescript + const s = new MyStorage(rootDir) + await s.init() + assert(typeof s.acquireWriterLock === 'function') + assert(s.supportsMultiProcessLocking() === true) + ``` + If `acquireWriterLock` is undefined the prototype chain is broken at + install time — fix install, not your plugin. + +6. **Pin your peer dep generously.** `"peerDependencies": { + "@soulcraft/brainy": "^7.21.0" }` accepts any compatible 7.x. Don't pin + to an exact patch unless you're tracking a known regression. + +## Future direction + +The 7 multi-process methods are currently defaults on `BaseStorage`. A +future refactor may extract them into a `MultiProcessSafeStorage` +interface/mixin for cleaner separation — only adapters that opt in would +expose them. This would require a minor bump and is tracked as an internal +follow-up; consumers don't need to anticipate the change. + +## Reading material + +- [`concepts/multi-process`](./multi-process.md) — the writer-lock model, + heartbeat semantics, what the lock protects. +- [`guides/inspection`](../guides/inspection.md) — `brainy inspect` and the + read-only mode. +- `node_modules/@soulcraft/brainy/dist/storage/baseStorage.d.ts` — the + authoritative type signatures for every method this page references. diff --git a/src/brainy.ts b/src/brainy.ts index 84668012..4bbc0aaf 100644 --- a/src/brainy.ts +++ b/src/brainy.ts @@ -303,14 +303,35 @@ export class Brainy implements BrainyInterface { } /** - * Whether the active storage adapter implements a given optional method. + * Whether the active storage adapter (anywhere in its prototype chain) + * implements a given optional method. * - * Brainy ships new storage-adapter capabilities over time (multi-process - * locking, cross-process flush RPC, etc.). Older plugins (notably - * `@soulcraft/cortex` ≤2.2.0) bundle their own `BaseStorage` from an - * earlier Brainy version that doesn't have those methods. Rather than crash - * at runtime, every call site that uses an optional storage method goes - * through this helper and degrades gracefully when the method is absent. + * Storage-adapter plugins (e.g. `@soulcraft/cortex`'s `MmapFileSystemStorage + * extends FileSystemStorage`) inherit new methods Brainy adds to + * `FileSystemStorage` / `BaseStorage` automatically — `typeof` walks the + * prototype chain, so there's no in-package version skew to worry about as + * long as the plugin's own dist resolves `@soulcraft/brainy` dynamically + * (which Cortex 2.2.x onward does — see + * `node_modules/@soulcraft/cortex/dist/storage/mmapFileSystemStorage.js`). + * + * This helper exists for the **build/install** failure modes the import + * resolution can't catch: + * - Stale `node_modules` left over from a prior `bun install` against + * `@soulcraft/brainy ≤7.20.x`. + * - Lockfile drift pinning brainy below the version that introduced the + * method. + * - Docker layer caches that reuse a `node_modules` from an earlier image. + * - Bundlers (esbuild, webpack) that freeze the prototype chain at build + * time and lose later prototype mutations. + * In any of those, calling the new method unconditionally crashes boot with + * `TypeError: storage.X is not a function`. The guard turns that into a + * loud warning + graceful degradation; the operator's clue is the warning + * naming the adapter class so they can re-run install / rebuild the image. + * + * Storage-adapter authors: see `docs/concepts/storage-adapters.md` for the + * inheritance contract. Filesystem-backed adapters extending + * `FileSystemStorage` inherit the 6 multi-process helpers for free; just + * override `supportsMultiProcessLocking()` → `true` to activate enforcement. */ private hasStorageMethod(name: string): boolean { return !!this.storage && typeof (this.storage as unknown as Record)[name] === 'function' @@ -422,11 +443,19 @@ export class Brainy implements BrainyInterface { if (backendName === 'MemoryStorage') { // Memory is single-process by construction — no warning needed. } else if (!this.hasStorageMethod('supportsMultiProcessLocking')) { + // The multi-process methods are inherited from FileSystemStorage + // when an adapter extends it. Reaching this branch means the + // prototype chain doesn't resolve to a Brainy version that + // defines them — almost always a build/install artifact rather + // than the plugin lacking the code. Tell the operator what to + // try first. console.warn( - `[brainy] Storage adapter \`${backendName}\` predates the 7.21 ` + - `multi-process methods. Writer locking and the flush-request RPC ` + - `are disabled for this directory. Upgrade the plugin (e.g. ` + - `\`@soulcraft/cortex\` ≥2.3.0) for full enforcement.` + `[brainy] Storage adapter \`${backendName}\` is missing the ` + + `multi-process methods on its prototype chain. Writer locking ` + + `and the flush-request RPC are disabled for this directory. ` + + `Likely fix: clean install (\`rm -rf node_modules bun.lockb && ` + + `bun install\`) or rebuild your container image to refresh ` + + `\`@soulcraft/brainy\` to ≥7.21. See docs/concepts/storage-adapters.md.` ) } else { console.warn(