diff --git a/src/brainy.ts b/src/brainy.ts index 0a2bd460..9344ff0c 100644 --- a/src/brainy.ts +++ b/src/brainy.ts @@ -367,6 +367,15 @@ class InsertPreconditionExistsSignal extends Error { } } +/** + * @description The derived-index families a read may depend on. A read that + * consults none of them (a canonical-storage read: `get`, an entity + * enumeration, a VFS content/dir read) is index-independent and must never + * block on another family's one-time migration. Used by the family-scoped + * migration gate ({@link Brainy.awaitMigrationLock}). + */ +export type IndexFamily = 'vector' | 'metadata' | 'graph' + /** * The main Brainy class - Clean, Beautiful, Powerful * REAL IMPLEMENTATION - No stubs, no mocks @@ -1439,7 +1448,10 @@ export class Brainy implements BrainyInterface { * re-initializing — using a closed Brainy is a consumer bug, not a lazy-init * opportunity. */ - private async ensureInitialized(opts?: { bypassMigrationLock?: boolean }): Promise { + private async ensureInitialized(opts?: { + bypassMigrationLock?: boolean + needs?: IndexFamily[] + }): Promise { if (this.closed) { throw new Error('Brainy instance is not initialized: it was closed via close(). Create a new instance.') } @@ -1449,12 +1461,17 @@ export class Brainy implements BrainyInterface { // Coordinated migration LOCK (#18): every data-plane read and write funnels // through here, so this is the single choke point that holds operations while // a native provider runs its one-time 7.x → 8.0 rebuild-from-canonical — no - // op touches a half-built index. Observability (`health`/`checkHealth`) and - // the lock-clearing path (`stampBrainFormat`, which does not route through - // here) opt out so an operator can always watch progress and cor can stamp. + // op touches a half-built index. The gate is FAMILY-SCOPED: `needs` names the + // derived-index families this operation actually consults, so a read served + // entirely from canonical storage (`needs: []`) or from a healthy family + // never blocks on an UNRELATED family's migration. `needs` omitted = the + // conservative whole-brain wait (writes, and any read not yet classified). + // Observability (`health`/`checkHealth`) and the lock-clearing path + // (`stampBrainFormat`, which does not route through here) opt out entirely so + // an operator can always watch progress and a native provider can stamp. // A brain that never migrates pays one boolean check (see awaitMigrationLock). if (!opts?.bypassMigrationLock) { - await this.awaitMigrationLock() + await this.awaitMigrationLock(opts?.needs) } } @@ -2188,7 +2205,9 @@ export class Brainy implements BrainyInterface { * */ async get(id: string, options?: GetOptions): Promise | null> { - await this.ensureInitialized() + // Canonical read: a get resolves an entity by id straight from storage and + // consults no derived index — it must not wait on any family's migration. + await this.ensureInitialized({ needs: [] }) this.warnIfReadsDegraded('get') // Id normalization (8.0): a caller may read by their natural key — resolve @@ -2247,7 +2266,8 @@ export class Brainy implements BrainyInterface { * ``` */ async batchGet(ids: string[], options?: GetOptions): Promise>> { - await this.ensureInitialized() + // Canonical read (see get): resolves by id from storage, no derived index. + await this.ensureInitialized({ needs: [] }) // Id normalization (8.0): resolve each id to its canonical UUID so callers // can batch-read by natural key. resolveEntityId is idempotent on real @@ -4271,7 +4291,9 @@ export class Brainy implements BrainyInterface { async related( paramsOrId?: string | RelatedParams ): Promise[]> { - await this.ensureInitialized() + // Graph read: relationship traversal consults only the graph adjacency + // family, so it waits on a graph migration but not on vector/metadata. + await this.ensureInitialized({ needs: ['graph'] }) await this.verifyGraphAdjacencyLive() // Handle string ID shorthand: related(id) -> related({ from: id }) @@ -5810,7 +5832,11 @@ export class Brainy implements BrainyInterface { * }) */ async find(query: string | FindParams): Promise[]> { - await this.ensureInitialized() + // Init only here — the migration gate is applied family-scoped below, once + // the query params reveal which index families this read actually consults + // (a string query may parse to a where / connected / vector shape). The lazy + // loader and cold-read probes below already defer to a migrating provider. + await this.ensureInitialized({ needs: [] }) // Ensure indexes are loaded (lazy loading when disableAutoRebuild: true) // This is a production-safe, concurrency-controlled lazy load @@ -5849,6 +5875,13 @@ export class Brainy implements BrainyInterface { // Zero-config validation (static import for performance) validateFindParams(params) + // Family-scoped migration gate: wait only on the index families THIS query + // consults, so a filter or graph query served from a healthy family never + // blocks on an unrelated family's one-time 7.x → 8.0 migration. Placed once + // params are final (after natural-language parse + connected-id resolution) + // and before the aggregate path, which carries no gate of its own. + await this.awaitMigrationLock(this.queryIndexFamilies(params)) + // Aggregate query path — early return when params.aggregate is set if (params.aggregate) { return this.findAggregate(params) @@ -12530,14 +12563,14 @@ export class Brainy implements BrainyInterface { entityCount: number indexEntryCount: number recommendation: string | null - /** Cross-layer (ADR-004 §6): each provider's own invariant self-report, when + /** Cross-layer: each provider's own invariant self-report, when * it exposes validateInvariants(). Absent providers are simply not listed. */ providers?: ProviderInvariantReport[] }> { await this.ensureInitialized() const result = await this.metadataIndex.validateConsistency() - // Cross-layer integrity (ADR-004 §6): validateConsistency() above only sees the + // Cross-layer integrity: validateConsistency() above only sees the // JS metadata index — it is BLIND to a native provider whose manifest ↔ // segments ↔ counts have diverged. Delegate to each provider's own // validateInvariants() and aggregate, so "healthy-while-broken" is impossible. @@ -12583,7 +12616,7 @@ export class Brainy implements BrainyInterface { /** * @description Feature-detect + call each index provider's OPTIONAL - * `validateInvariants()` (ADR-004 §6) and collect the reports. The contract is + * `validateInvariants()` and collect the reports. The contract is * that `validateInvariants()` NEVER throws and is bounded <50ms — but if a * provider violates that, the throw is turned into an UNHEALTHY synthetic * report (loud), never swallowed into "healthy". Providers that do not expose @@ -12673,7 +12706,8 @@ export class Brainy implements BrainyInterface { limit?: number } ): Promise { - await this.ensureInitialized() + // Graph read (see related): adjacency traversal only. + await this.ensureInitialized({ needs: ['graph'] }) await this.verifyGraphAdjacencyLive() const direction = options?.direction || 'both' @@ -14844,6 +14878,58 @@ export class Brainy implements BrainyInterface { ) } + /** + * @description The provider instance backing a given index {@link IndexFamily}. + * The single place the family label maps to the concrete provider, so the + * scoped migration gate and any future family-aware routing agree. + */ + private providerForFamily(family: IndexFamily): unknown { + switch (family) { + case 'vector': + return this.index + case 'metadata': + return this.metadataIndex + case 'graph': + return this.graphIndex + } + } + + /** + * @description Whether the migration LOCK should hold for an operation that + * depends on the given index families: + * - `needs === undefined` → WHOLE-BRAIN: any migrating provider counts (the + * conservative default for writes and unclassified reads — a write touches + * every index). + * - `needs === []` → NEVER: a canonical-storage read consults no derived index, + * so a migration elsewhere is irrelevant; it serves immediately. + * - `needs = [families]` → SCOPED: only those families' providers count, so a + * read served from a healthy family is not blocked by an unrelated family's + * one-time migration. + */ + private neededFamiliesMigrating(needs?: IndexFamily[]): boolean { + if (needs === undefined) return this.anyProviderMigrating() + for (const family of needs) { + if (this.providerIsMigrating(this.providerForFamily(family))) return true + } + return false + } + + /** + * @description The index families a {@link find} query consults, derived from + * its params — the read-side input to the family-scoped migration gate. A + * vector / near / semantic query needs `vector`; a `where` / type / subtype / + * service filter or an aggregate needs `metadata`; a `connected` traversal + * needs `graph`. A query combining shapes needs the union. Mirrors find()'s + * own criteria split so the gate and the executor never disagree. + */ + private queryIndexFamilies(params: FindParams): IndexFamily[] { + const needs: IndexFamily[] = [] + if ((params.query && params.query.trim() !== '') || params.vector || params.near) needs.push('vector') + if (params.where || params.type || params.subtype || params.service || params.aggregate) needs.push('metadata') + if (params.connected) needs.push('graph') + return needs + } + /** * @description Rich migration progress relayed verbatim from whichever provider * exposes the OPTIONAL `migrationStatus(): { phase?, index?, percent?, … } | null`. @@ -14872,11 +14958,19 @@ export class Brainy implements BrainyInterface { /** * @description Block-and-queue on the coordinated migration LOCK (#18). Called * at the {@link ensureInitialized} choke point (and once inside `init()` before - * VFS bootstrap), so every data-plane read and write — and startup itself — - * waits here while a native provider runs its one-time 7.x → 8.0 - * rebuild-from-canonical. The caller gets the correct answer, never a partial - * read of a half-built index and never a lost write. A brain that is not - * migrating pays a single boolean check and returns immediately. + * VFS bootstrap), so a data-plane operation waits here while a native provider + * runs its one-time 7.x → 8.0 rebuild-from-canonical. The caller gets the + * correct answer, never a partial read of a half-built index and never a lost + * write. A brain that is not migrating pays a single boolean check and returns + * immediately. + * + * FAMILY-SCOPED: `needs` names the index families the caller depends on, so the + * wait holds ONLY for a migration of one of those families. A read served + * entirely from canonical storage (`needs: []`) or from a healthy family never + * blocks on an unrelated family's migration. `needs` omitted = the conservative + * whole-brain wait (writes touch every index; startup wants all of them ready). + * @param needs - Index families this operation consults, or `undefined` for the + * whole-brain wait. * * IMPORTANT: this timeout bounds THE CALLER'S WAIT, not the migration. The * migration itself is unbounded — the native provider rebuilds a billion-scale @@ -14890,8 +14984,8 @@ export class Brainy implements BrainyInterface { * for a very large brain, runs the offline migrator. The block/release lines * log once per window; the flags reset on release so a later upgrade re-logs. */ - private async awaitMigrationLock(): Promise { - if (!this.anyProviderMigrating()) return // fast path — the overwhelming common case + private async awaitMigrationLock(needs?: IndexFamily[]): Promise { + if (!this.neededFamiliesMigrating(needs)) return // fast path — the overwhelming common case const startedAt = Date.now() const timeoutMs = this.config.migrationWaitTimeoutMs ?? 30_000 @@ -14904,7 +14998,7 @@ export class Brainy implements BrainyInterface { ) } - while (this.anyProviderMigrating()) { + while (this.neededFamiliesMigrating(needs)) { const remaining = timeoutMs - (Date.now() - startedAt) if (remaining <= 0) { const status = this.providerMigrationStatus() @@ -15092,6 +15186,13 @@ export class Brainy implements BrainyInterface { */ private async ensureMetadataConsistencyProbed(): Promise { if (this._metadataConsistencyProbed) return + // Defer while the metadata provider runs its one-time in-place migration: + // probing (and self-healing via rebuild) an index the provider is mid-rebuild + // would collide with the provider that owns it. Mirrors the vector deference + // in ensureIndexesLoaded. Do NOT latch — once the migration clears, the next + // read runs the probe. (The family-scoped find() gate waits on the metadata + // family separately before any actual filter read.) + if (this.providerIsMigrating(this.metadataIndex)) return this._metadataConsistencyProbed = true const provider = this.metadataIndex as { probeConsistency?: () => Promise @@ -15161,6 +15262,32 @@ export class Brainy implements BrainyInterface { async repairIndex(): Promise { await this.ensureInitialized() + + // Prune orphaned canonical containers left by the pre-8.3.1 partial-delete + // defect: a delete that removed the metadata (content) leg but left the + // vector leg + the entity directory (a "ghost"), or left an empty directory + // (a "scar"). These are not live entities (getNoun needs the content leg) + // yet inflate enumerated counts and confuse locator resolution. Feature- + // detected (filesystem-only — key/prefix stores have no orphan containers); + // recompute counts afterward so the totals stop counting the ghosts. + const pruner = this.storage as { + pruneOrphanedEntities?: () => Promise<{ nouns: string[]; verbs: string[] }> + rebuildTypeCounts?: () => Promise + rebuildSubtypeCounts?: () => Promise + } + if (typeof pruner.pruneOrphanedEntities === 'function') { + const orphans = await pruner.pruneOrphanedEntities() + if (orphans.nouns.length + orphans.verbs.length > 0) { + await pruner.rebuildTypeCounts?.() + await pruner.rebuildSubtypeCounts?.() + prodLog.warn( + `[Brainy] repairIndex() pruned ${orphans.nouns.length} orphaned noun + ` + + `${orphans.verbs.length} orphaned verb container(s) left by a pre-8.3.1 ` + + `partial delete, and recomputed counts.` + ) + } + } + await this.metadataIndex.detectAndRepairCorruption() // Lift a failed-rollback write-quarantine: force a full rebuild so the // derived indexes are provably reconciled with canonical, then clear the @@ -15175,7 +15302,7 @@ export class Brainy implements BrainyInterface { `Writes are re-enabled.` ) } - // Cross-layer repair (ADR-004 §6): repairIndex must reconcile NATIVE derived + // Cross-layer repair: repairIndex must reconcile NATIVE derived // state from canonical, not just the JS metadata index. Consult each provider's // own validateInvariants() and rebuild any whose failing invariant asks for it // (heal: 'rebuild') — the native counterpart of detectAndRepairCorruption(). diff --git a/src/storage/adapters/fileSystemStorage.ts b/src/storage/adapters/fileSystemStorage.ts index c2352d2f..a9e3014c 100644 --- a/src/storage/adapters/fileSystemStorage.ts +++ b/src/storage/adapters/fileSystemStorage.ts @@ -502,6 +502,104 @@ export class FileSystemStorage extends BaseStorage { this.writeBarrierDeleteDirs?.add(path.dirname(pathStr)) } + /** + * @description Remove the entity container directory after both canonical legs + * were deleted (full-removal delete). `objectLegPath` is one leg's + * storage-root-relative path (e.g. `entities/nouns///vectors.json`); + * its parent is the `/` entity directory. `rmdir` removes it ONLY if empty + * — both legs are gone by this point, so it should be. A non-empty dir means an + * unexpected leftover (a leg whose delete faulted — already surfaced upstream — + * or foreign data): we do NOT recursively nuke it (loud errors, never quiet + * losses); we log and leave it for the orphan repair sweep (`repairIndex`). + */ + protected override async removeCanonicalContainer(objectLegPath: string): Promise { + const relDir = path.dirname(objectLegPath) + const absDir = path.join(this.rootDir, relDir) + try { + await fs.promises.rmdir(absDir) + // The dir removal is durable once its PARENT (the shard dir) is fsync'd. + this.writeBarrierDeleteDirs?.add(path.dirname(relDir)) + } catch (error: any) { + if (error?.code === 'ENOENT') return // container already gone — fine + if (error?.code === 'ENOTEMPTY' || error?.code === 'EEXIST') { + console.warn( + `[FileSystemStorage] entity container ${relDir} not empty after delete — ` + + `leaving it for the orphan repair sweep (brain.repairIndex()).` + ) + return + } + throw error + } + } + + /** + * @description Prune orphaned entity directories left by the pre-8.3.1 + * partial-delete defect. A delete used to remove the metadata (content) leg + * but leave the `vectors.json` leg + the `/` directory (a "ghost"), or — + * on the direct storage path — remove both legs but leave an empty directory + * (a "scar"). Neither is a live entity (`getNoun` needs the metadata content + * leg) yet each lingers on disk, inflating enumerated counts and confusing + * locator resolution. + * + * CONSERVATIVE by design: removes ONLY dirs with NO metadata content leg — a + * directory that still holds its content is left untouched. LOUD: logs every + * removed orphan. Operator-invoked via {@link Brainy.repairIndex}; never runs + * automatically. Returns the pruned container ids so the caller can recompute + * counts. + */ + public async pruneOrphanedEntities(): Promise<{ nouns: string[]; verbs: string[] }> { + await this.ensureInitialized() + const pruned: { nouns: string[]; verbs: string[] } = { nouns: [], verbs: [] } + + for (const [kind, root] of [ + ['nouns', 'entities/nouns'], + ['verbs', 'entities/verbs'] + ] as const) { + const rootAbs = path.join(this.rootDir, root) + let shards: string[] + try { + shards = await fs.promises.readdir(rootAbs) + } catch (error: any) { + if (error?.code === 'ENOENT') continue // no entities of this kind yet + throw error + } + + for (const shard of shards) { + const shardAbs = path.join(rootAbs, shard) + let entries: import('fs').Dirent[] + try { + entries = await fs.promises.readdir(shardAbs, { withFileTypes: true }) + } catch (error: any) { + if (error?.code === 'ENOENT') continue + throw error + } + + for (const entry of entries) { + if (!entry.isDirectory()) continue + const idAbs = path.join(shardAbs, entry.name) + let legs: string[] + try { + legs = await fs.promises.readdir(idAbs) + } catch (error: any) { + if (error?.code === 'ENOENT') continue + throw error + } + // A live entity has its metadata content leg. No content leg → a + // vector-only ghost or an empty scar → prune the whole container. + if (legs.some((f) => f.startsWith('metadata.json'))) continue + await fs.promises.rm(idAbs, { recursive: true, force: true }) + pruned[kind].push(entry.name) + console.warn( + `[FileSystemStorage] pruned orphaned ${kind === 'nouns' ? 'noun' : 'verb'} ` + + `container ${root}/${shard}/${entry.name} (no metadata content leg)` + ) + } + } + } + + return pruned + } + /** * Primitive operation: List objects under path prefix * All metadata operations use this internally via base class routing @@ -631,7 +729,7 @@ export class FileSystemStorage extends BaseStorage { public override async removeRawPrefix(prefix: string): Promise { await this.ensureInitialized() // Registered-blob contract: a prefix-nuke must not take out a protected - // family member (ADR-004 §7). Throws if the prefix intersects one. + // family member. Throws if the prefix intersects one. await this.assertPrefixNotProtected(prefix) await fs.promises.rm(path.join(this.rootDir, prefix), { recursive: true, force: true }) } @@ -1240,7 +1338,7 @@ export class FileSystemStorage extends BaseStorage { // errors, never quiet losses. Fault-propagation restored lockstep with // cortex 3.0.13, whose two column-store call sites now handle the throw // (they mark the field unavailable + throw a named error) instead of - // relying on null-on-error (CORTEX-BILLION-SCALE-SPINE). + // relying on null-on-error. if (isAbsentError(err)) return null throw err } @@ -1253,7 +1351,7 @@ export class FileSystemStorage extends BaseStorage { */ public async deleteBinaryBlob(key: string): Promise { await this.ensureInitialized() - // Registered-blob contract (ADR-004 §7): refuse to delete a declared + // Registered-blob contract: refuse to delete a declared // derived-index family member — an in-process GC/sweeper cannot remove a // load-bearing index file. Throws ProtectedArtifactError; no-op when no // families are registered. diff --git a/src/storage/baseStorage.ts b/src/storage/baseStorage.ts index 5fdf129f..f5efdcb6 100644 --- a/src/storage/baseStorage.ts +++ b/src/storage/baseStorage.ts @@ -253,7 +253,7 @@ export abstract class BaseStorage extends BaseStorageAdapter { /** One-shot guard so the graph fast-path cold-load probe runs once per adapter. */ private _graphFastPathProbed = false /** - * Registered-blob contract (ADR-004 §7): declared derived-index families, keyed + * Registered-blob contract: declared derived-index families, keyed * by name. Members are undeletable through the blob delete seams. Loaded lazily * from `_system/derived-artifacts.json` and re-persisted on every change. */ @@ -842,6 +842,24 @@ export abstract class BaseStorage extends BaseStorageAdapter { return this.deleteObjectFromPath(path) } + /** + * @description Remove the container that held a canonical entity's leg files, + * called after both legs are deleted so a delete leaves NOTHING behind — no + * orphan "scar" directory. `objectLegPath` is any one of the entity's leg + * paths (e.g. its `vectors.json`); the container is that path's parent. + * + * Default: a no-op. Key/prefix-addressed stores (in-memory, cloud object + * stores) have no empty-container concept — deleting the leg keys already + * removes the entity entirely. The filesystem adapter overrides this to + * `rmdir` the emptied entity directory. + * + * @param objectLegPath - Storage-root-relative path of one of the entity's legs. + * @protected + */ + protected async removeCanonicalContainer(objectLegPath: string): Promise { + void objectLegPath + } + /** * @description List canonical objects under a storage-root-relative prefix. * @@ -1062,7 +1080,7 @@ export abstract class BaseStorage extends BaseStorageAdapter { } // ========================================================================== - // Registered-blob contract (ADR-004 §7) — declared derived-index families are + // Registered-blob contract — declared derived-index families are // undeletable through the blob delete seams. Shared here so every adapter that // extends BaseStorage inherits the same enforcement; the concrete // deleteBinaryBlob / removeRawPrefix call assertBlobKeyDeletable / @@ -1116,7 +1134,7 @@ export abstract class BaseStorage extends BaseStorageAdapter { /** * @description A blob key that is a transient write-scratch file (a `*.tmp.*` * temp, a `*.rebuild-tmp`, a `*.rotate-tmp`). Its OWNER renames/removes it as - * part of an atomic write; it is never a protected family member (ADR-004 §7). + * part of an atomic write; it is never a protected family member. */ private isTransientBlobKey(key: string): boolean { return /\.rebuild-tmp$|\.rotate-tmp$|\.tmp(\.|$)/.test(key) @@ -1185,7 +1203,7 @@ export abstract class BaseStorage extends BaseStorageAdapter { /** * @description Verify every declared family has all its members present on - * disk (ADR-004 §7 missing-on-open catch for an EXTERNAL deleter that bypasses + * disk (missing-on-open catch for an EXTERNAL deleter that bypasses * the in-process refusal). Returns the incomplete families (name + missing * members) — the caller decides how to heal (rebuild from canonical). Loud by * construction: a missing load-bearing blob is named, never silently tolerated. @@ -1601,16 +1619,22 @@ export abstract class BaseStorage extends BaseStorageAdapter { public async deleteNoun(id: string): Promise { await this.ensureInitialized() - // Delete both the vector file and metadata file (2-file system) - await this.deleteNoun_internal(id) + // FULL removal (live-HEAD hygiene): remove BOTH canonical legs AND the + // entity's container, so a delete leaves nothing behind — no orphan/scar + // directory to inflate the enumerated count or ghost resolveLocator. The + // generation store holds the immutable before-image, so asOf() still + // reconstructs the deleted entity until retention expires. + await this.deleteNoun_internal(id) // vectors.json leg - // Delete metadata file (if it exists) - try { - await this.deleteNounMetadata(id) - } catch (error) { - // Ignore if metadata file doesn't exist - prodLog.debug(`No metadata file to delete for noun ${id}`) - } + // Metadata leg + count decrement. A genuine "already absent" is a no-op + // downstream (readCanonicalObject / deleteObjectFromPath both treat ENOENT as + // success); a REAL fault must surface loudly (loud errors, never quiet + // losses) and must never silently skip the count decrement — so this is NO + // LONGER wrapped in a blind catch that masked faults as "file didn't exist". + await this.deleteNounMetadata(id) + + // Remove the now-empty entity container (a no-op for key/prefix stores). + await this.removeCanonicalContainer(getNounVectorPath(id)) } /** @@ -2915,16 +2939,12 @@ export abstract class BaseStorage extends BaseStorageAdapter { public async deleteVerb(id: string): Promise { await this.ensureInitialized() - // Delete both the vector file and metadata file (2-file system) - await this.deleteVerb_internal(id) - - // Delete metadata file (if it exists) - try { - await this.deleteVerbMetadata(id) - } catch (error) { - // Ignore if metadata file doesn't exist - prodLog.debug(`No metadata file to delete for verb ${id}`) - } + // FULL removal (see deleteNoun): both canonical legs + the verb container. + // The blind catch that masked real faults as "no metadata file" is gone — a + // genuine absence is already a no-op downstream; a real fault surfaces loudly. + await this.deleteVerb_internal(id) // vectors.json leg + await this.deleteVerbMetadata(id) // metadata leg + count decrement + await this.removeCanonicalContainer(getVerbVectorPath(id)) } /** * Get graph index (lazy initialization with concurrent access protection) diff --git a/src/transaction/operations/StorageOperations.ts b/src/transaction/operations/StorageOperations.ts index ca513cd4..bd753552 100644 --- a/src/transaction/operations/StorageOperations.ts +++ b/src/transaction/operations/StorageOperations.ts @@ -104,13 +104,26 @@ export class SaveNounOperation implements Operation { } /** - * Delete noun metadata with rollback support + * Delete a noun — FULL canonical removal, with rollback support. + * + * Despite the historical name, this removes the WHOLE entity: both canonical + * legs (metadata + vector) AND the entity's container. Previously it deleted + * only the metadata leg via `deleteNounMetadata`, leaving the canonical + * `vectors.json` leg and the `/` directory orphaned on disk — a "ghost" + * that reads as absent (getNoun needs both legs) yet inflates the enumerated + * count and can never be told apart from a damage scar. Routing through + * `storage.deleteNoun` removes both legs + the container in one place. + * + * Immutability is preserved: this cleans only the live-HEAD projection; the + * generation store retains the before-image so `asOf()` still reconstructs the + * deleted entity until retention expires. * * Rollback strategy: - * - Restore deleted metadata + * - Restore BOTH legs from the before-image (vector leg raw, metadata leg via + * the count-aware save so deleteNoun()'s decrement is reversed). */ export class DeleteNounMetadataOperation implements Operation { - readonly name = 'DeleteNounMetadata' + readonly name = 'DeleteNoun' constructor( private readonly storage: StorageAdapter, @@ -118,21 +131,34 @@ export class DeleteNounMetadataOperation implements Operation { ) {} async execute(): Promise { - // Get metadata before deletion (for rollback) + // Capture the FULL before-image (both legs) so the undo restores the whole + // entity — a metadata-only rollback would leave the vector leg unrestored. + const previousNoun = await this.storage.getNoun(this.id) const previousMetadata = await this.storage.getNounMetadata(this.id) - if (!previousMetadata) { + if (!previousNoun && !previousMetadata) { // Nothing to delete - no rollback needed return async () => {} } - // Delete metadata - await this.storage.deleteNounMetadata(this.id) + // Full removal: both canonical legs + the entity container + count decrement. + await this.storage.deleteNoun(this.id) // Return rollback action return async () => { - // Restore deleted metadata - await this.storage.saveNounMetadata(this.id, previousMetadata) + // Restore the vector leg, then the metadata leg through the count-aware + // save so deleteNoun()'s decrement is reversed. + if (previousNoun) { + await this.storage.saveNoun({ + id: previousNoun.id, + vector: previousNoun.vector, + connections: previousNoun.connections || new Map(), + level: previousNoun.level || 0 + }) + } + if (previousMetadata) { + await this.storage.saveNounMetadata(this.id, previousMetadata) + } } } } diff --git a/tests/integration/delete-full-removal.test.ts b/tests/integration/delete-full-removal.test.ts new file mode 100644 index 00000000..83675a13 --- /dev/null +++ b/tests/integration/delete-full-removal.test.ts @@ -0,0 +1,161 @@ +/** + * @module tests/integration/delete-full-removal + * @description Canonical noun/verb deletes are FULL removals: both legs + * (metadata + vectors) AND the entity's `/` container are removed, so a + * delete leaves NOTHING behind. Regression for the ghost/scar defect where + * remove() deleted the vector INDEX entry + the canonical metadata leg but never + * the canonical vectors.json leg or the directory — leaving an orphan that reads + * as absent (getNoun needs both legs) yet inflated the enumerated count and + * confused locator resolution. Also covers the operator repair sweep + * (brain.repairIndex()) that prunes orphans left by the pre-fix behavior. + */ +import { describe, it, expect, beforeEach, afterEach } from 'vitest' +import * as fs from 'node:fs' +import * as os from 'node:os' +import * as path from 'node:path' +import { Brainy } from '../../src/index.js' + +/** All entity-directory names (the `` dirs) under entities/. */ +function entityDirs(root: string, kind: 'nouns' | 'verbs'): string[] { + const base = path.join(root, 'entities', kind) + if (!fs.existsSync(base)) return [] + const ids: string[] = [] + for (const shard of fs.readdirSync(base)) { + const shardDir = path.join(base, shard) + if (!fs.statSync(shardDir).isDirectory()) continue + for (const id of fs.readdirSync(shardDir)) { + if (fs.statSync(path.join(shardDir, id)).isDirectory()) ids.push(id) + } + } + return ids +} + +/** Absolute path of one entity's `` directory (or null if not present). */ +function entityDir(root: string, kind: 'nouns' | 'verbs', id: string): string | null { + const base = path.join(root, 'entities', kind) + if (!fs.existsSync(base)) return null + for (const shard of fs.readdirSync(base)) { + const candidate = path.join(base, shard, id) + if (fs.existsSync(candidate)) return candidate + } + return null +} + +describe('canonical delete is a full removal (no ghost/scar directory)', () => { + let dir: string + let brain: any + + beforeEach(async () => { + process.env.BRAINY_DETERMINISTIC_EMBEDDINGS = 'true' + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'brainy-del-')) + brain = new Brainy({ requireSubtype: false, storage: { type: 'filesystem', path: dir }, silent: true, dimensions: 384 }) + await brain.init() + }) + afterEach(async () => { + await brain.close?.().catch(() => {}) + fs.rmSync(dir, { recursive: true, force: true }) + }) + + it('remove() deletes BOTH legs and the container — nothing left on disk', async () => { + const ids: string[] = [] + for (let i = 0; i < 3; i++) ids.push(await brain.add({ data: `doc ${i}`, type: 'document', metadata: { i } })) + await brain.flush() + + // (A filesystem brain also has its VFS-root noun, so don't assume an exact set.) + const before = entityDirs(dir, 'nouns') + expect(before).toEqual(expect.arrayContaining(ids)) + + await brain.remove(ids[1]) + await brain.flush() + + // getNoun is null AND the on-disk container is fully gone (no orphan), and + // EXACTLY one directory disappeared (the removed entity's). + expect(await brain.get(ids[1])).toBeNull() + expect(entityDir(dir, 'nouns', ids[1])).toBeNull() + const after = entityDirs(dir, 'nouns') + expect(after).toHaveLength(before.length - 1) + expect(after).toEqual(expect.arrayContaining([ids[0], ids[2]])) + expect(after).not.toContain(ids[1]) + }) + + it('the enumerated count is honest after a delete (no monotonic inflation)', async () => { + const ids: string[] = [] + for (let i = 0; i < 4; i++) ids.push(await brain.add({ data: `n${i}`, type: 'document', metadata: { i } })) + await brain.flush() + + const before = await brain.storage.getNounsWithPagination({ limit: 100, offset: 0 }) + + await brain.remove(ids[0]) + await brain.remove(ids[1]) + await brain.flush() + + // The count drops by EXACTLY the two removed entities — no ghost lingers to + // hold the total up (the pre-fix defect left it monotonic). + const after = await brain.storage.getNounsWithPagination({ limit: 100, offset: 0 }) + expect(after.totalCount).toBe(before.totalCount - 2) + const remaining = after.items.map((n: any) => n.id) + expect(remaining).toEqual(expect.arrayContaining([ids[2], ids[3]])) + expect(remaining).not.toContain(ids[0]) + expect(remaining).not.toContain(ids[1]) + }) +}) + +describe('repairIndex() prunes orphan containers left by the pre-fix partial delete', () => { + let dir: string + let brain: any + + beforeEach(async () => { + process.env.BRAINY_DETERMINISTIC_EMBEDDINGS = 'true' + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'brainy-orphan-')) + brain = new Brainy({ requireSubtype: false, storage: { type: 'filesystem', path: dir }, silent: true, dimensions: 384 }) + await brain.init() + }) + afterEach(async () => { + await brain.close?.().catch(() => {}) + fs.rmSync(dir, { recursive: true, force: true }) + }) + + it('a vector-only "ghost" (metadata leg deleted out-of-band) is pruned', async () => { + const ids: string[] = [] + for (let i = 0; i < 3; i++) ids.push(await brain.add({ data: `g${i}`, type: 'document', metadata: { i } })) + await brain.flush() + + // Simulate the pre-fix defect on ids[0]: remove ONLY its metadata leg, + // leaving vectors.json + the directory (the exact ghost shape). + const ghostDir = entityDir(dir, 'nouns', ids[0])! + for (const f of fs.readdirSync(ghostDir)) { + if (f.startsWith('metadata.json')) fs.rmSync(path.join(ghostDir, f)) + } + // The ghost dir (vectors.json, no metadata content leg) still sits on disk. + expect(entityDir(dir, 'nouns', ids[0])).not.toBeNull() + + await brain.repairIndex() + + // The sweep removes the ghost container; the two healthy entities survive. + expect(entityDir(dir, 'nouns', ids[0])).toBeNull() + expect(entityDir(dir, 'nouns', ids[1])).not.toBeNull() + expect(entityDir(dir, 'nouns', ids[2])).not.toBeNull() + }) + + it('an empty "scar" directory is pruned', async () => { + const id = await brain.add({ data: 'lonely', type: 'document', metadata: {} }) + await brain.flush() + const scarDir = entityDir(dir, 'nouns', id)! + for (const f of fs.readdirSync(scarDir)) fs.rmSync(path.join(scarDir, f)) // empty the dir, keep it + expect(fs.existsSync(scarDir)).toBe(true) + + await brain.repairIndex() + + expect(fs.existsSync(scarDir)).toBe(false) + }) + + it('a healthy entity (both legs present) is NEVER pruned', async () => { + const id = await brain.add({ data: 'keep me', type: 'document', metadata: { keep: true } }) + await brain.flush() + + await brain.repairIndex() + + expect(entityDir(dir, 'nouns', id)).not.toBeNull() + expect(await brain.get(id)).not.toBeNull() + }) +}) diff --git a/tests/integration/readdir-no-duplicate-replace.test.ts b/tests/integration/readdir-no-duplicate-replace.test.ts new file mode 100644 index 00000000..8043bd40 --- /dev/null +++ b/tests/integration/readdir-no-duplicate-replace.test.ts @@ -0,0 +1,56 @@ +/** + * @module tests/integration/readdir-no-duplicate-replace + * @description A re-created path must NOT show up twice in readdir. The reported + * defect (memory-vm) was readdir serving DUPLICATE entries for re-created paths, + * rooted in a delete that left a ghost (partial removal) whose next delete read + * null metadata and skipped unposting the stale Contains edge. With full-removal + * deletes there is no ghost, so the edge is always unposted and the path appears + * exactly once after any number of delete→recreate cycles. + */ +import { describe, it, expect, beforeEach, afterEach } from 'vitest' +import * as fs from 'node:fs' +import * as os from 'node:os' +import * as path from 'node:path' +import { Brainy } from '../../src/index.js' + +describe('readdir shows a re-created path exactly once (no duplicate on replace)', () => { + let dir: string + let brain: any + + beforeEach(async () => { + process.env.BRAINY_DETERMINISTIC_EMBEDDINGS = 'true' + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'brainy-readdir-')) + brain = new Brainy({ requireSubtype: false, storage: { type: 'filesystem', path: dir }, silent: true, dimensions: 384 }) + await brain.init() + }) + afterEach(async () => { + await brain.close?.().catch(() => {}) + fs.rmSync(dir, { recursive: true, force: true }) + }) + + it('delete then re-create the same file path — readdir lists it once', async () => { + await brain.vfs.mkdir('/d', { recursive: true }) + await brain.vfs.writeFile('/d/x.txt', 'v1') + expect(await brain.vfs.readdir('/d')).toEqual(['x.txt']) + + await brain.vfs.unlink('/d/x.txt') + expect(await brain.vfs.readdir('/d')).toEqual([]) + + await brain.vfs.writeFile('/d/x.txt', 'v2') + const listing = (await brain.vfs.readdir('/d')) as string[] + expect(listing).toEqual(['x.txt']) // exactly once — no ghost duplicate + expect(listing.filter((n) => n === 'x.txt')).toHaveLength(1) + }) + + it('survives several delete→recreate cycles without accumulating duplicates', async () => { + await brain.vfs.mkdir('/c', { recursive: true }) + for (let i = 0; i < 4; i++) { + await brain.vfs.writeFile('/c/f.txt', `gen ${i}`) + await brain.vfs.unlink('/c/f.txt') + } + await brain.vfs.writeFile('/c/f.txt', 'final') + const listing = (await brain.vfs.readdir('/c')) as string[] + expect(listing).toEqual(['f.txt']) + expect(await brain.vfs.readFile('/c/f.txt')).toBeDefined() + }) +}) diff --git a/tests/unit/brainy/migration-gate-family-scoped.test.ts b/tests/unit/brainy/migration-gate-family-scoped.test.ts new file mode 100644 index 00000000..b71c3899 --- /dev/null +++ b/tests/unit/brainy/migration-gate-family-scoped.test.ts @@ -0,0 +1,94 @@ +/** + * @module tests/unit/brainy/migration-gate-family-scoped + * @description The coordinated migration LOCK is FAMILY-SCOPED: a read waits only + * on the index families it actually consults. A one-time native migration of ONE + * family (its `isMigrating()` held) must NOT block a read served entirely from + * canonical storage (get / VFS content + dir) or from a HEALTHY family — only a + * read that needs the migrating family blocks. Regression for the whole-brain + * gate that hung getStats / readdir / readFile behind an unrelated family's + * migration until the wait timed out. + */ +import { describe, it, expect, beforeEach } from 'vitest' +import { Brainy } from '../../../src/brainy.js' +import { MigrationInProgressError } from '../../../src/errors/brainyError.js' + +const seed = async () => { + const brain = new Brainy({ + storage: { type: 'memory' }, + dimensions: 384, + requireSubtype: false, + silent: true, + // Short so a genuine block resolves fast; a real block would otherwise wait + // the 30 s default — this test asserts the wait does NOT happen for scoped + // reads, and DOES (bounded) for the one that needs the migrating family. + migrationWaitTimeoutMs: 300 + }) + await brain.init() + for (let i = 0; i < 5; i++) { + await brain.add({ data: `doc ${i}`, type: 'document', metadata: { i } }) + } + await brain.vfs.writeFile('/notes/hello.txt', 'canonical bytes — no index needed') + await brain.flush() + return brain +} + +/** Force a provider to report an in-flight (stuck) migration. */ +const jam = (provider: unknown) => { + ;(provider as { isMigrating: () => boolean }).isMigrating = () => true +} + +describe('migration LOCK is family-scoped', () => { + beforeEach(() => { + process.env.BRAINY_DETERMINISTIC_EMBEDDINGS = 'true' + }) + + it('a stuck VECTOR migration does not block canonical or graph/metadata reads', async () => { + const brain = await seed() + const childId = ( + (await brain.vfs.readdir('/notes', { withFileTypes: true })) as Array<{ entityId: string }> + )[0].entityId + + jam((brain as any).index) // vector provider migrating; graph + metadata healthy + + // None of these consult the vector family → they serve immediately. + await expect(brain.getStats()).resolves.toBeDefined() + await expect(brain.vfs.readdir('/notes')).resolves.toHaveLength(1) // graph traversal + await expect(brain.vfs.readFile('/notes/hello.txt')).resolves.toBeDefined() // canonical blob + await expect(brain.get(childId)).resolves.not.toBeNull() // canonical by id + await expect(brain.find({ where: { i: 1 } })).resolves.toBeDefined() // metadata filter + }) + + it('a stuck VECTOR migration STILL blocks a read that needs the vector family', async () => { + const brain = await seed() + jam((brain as any).index) + + // A semantic query consults the vector index — it must wait, and (bounded by + // migrationWaitTimeoutMs) surface the retryable MigrationInProgressError + // rather than serve a half-built result. + await expect(brain.find({ query: 'doc' })).rejects.toBeInstanceOf(MigrationInProgressError) + }) + + it('a stuck GRAPH migration blocks traversal but not vector/canonical reads', async () => { + const brain = await seed() + const childId = ( + (await brain.vfs.readdir('/notes', { withFileTypes: true })) as Array<{ entityId: string }> + )[0].entityId + + jam((brain as any).graphIndex) // graph provider migrating; vector + metadata healthy + + // Canonical + vector + metadata are unaffected. + await expect(brain.get(childId)).resolves.not.toBeNull() + await expect(brain.find({ query: 'doc' })).resolves.toBeDefined() + await expect(brain.find({ where: { i: 1 } })).resolves.toBeDefined() + + // A graph traversal needs the migrating family → it blocks (bounded). + await expect(brain.related({ from: childId })).rejects.toBeInstanceOf(MigrationInProgressError) + }) + + it('with no migration in flight, every read serves (the fast path is a no-op)', async () => { + const brain = await seed() + await expect(brain.getStats()).resolves.toBeDefined() + await expect(brain.find({ query: 'doc' })).resolves.toBeDefined() + await expect(brain.vfs.readdir('/notes')).resolves.toHaveLength(1) + }) +}) diff --git a/tests/unit/migration-lock.test.ts b/tests/unit/migration-lock.test.ts index 81d44a9f..f0fbbe4c 100644 --- a/tests/unit/migration-lock.test.ts +++ b/tests/unit/migration-lock.test.ts @@ -2,11 +2,16 @@ * Migration LOCK (#18) — coordinated, automatic 7.x → 8.0 auto-upgrade. * * Exercises the real block-and-queue behavior of `awaitMigrationLock` at the - * `ensureInitialized` choke point: while ANY index provider reports - * `isMigrating() === true`, data-plane reads and writes WAIT (no operation - * touches a half-built index); observability (`getIndexStatus`, `health`) and - * the lock-clearing `stampBrainFormat` stay exempt; and a lock that outlives the - * configured window surfaces a retryable `MigrationInProgressError`. + * `ensureInitialized` choke point. The gate is FAMILY-SCOPED: a write (which + * touches every index) WAITS while ANY provider reports `isMigrating() === true`, + * and a read WAITS only when the migrating provider is one of the index families + * that read consults (so a read served from canonical storage or a healthy family + * is never blocked by an unrelated family's migration — see + * `tests/unit/brainy/migration-gate-family-scoped.test.ts`). Observability + * (`getIndexStatus`, `health`) and the lock-clearing `stampBrainFormat` stay + * exempt; a lock that outlives the configured window surfaces a retryable + * `MigrationInProgressError`. Here the graph provider holds the lock, so the + * read cases use a graph traversal (`related`) — the family that actually waits. * * A native (cortex) provider owns the real migration; here we simulate the lock * by feature-detect-injecting `isMigrating()` onto a live provider, exactly as @@ -84,11 +89,12 @@ describe('Migration LOCK (#18) — coordinated 7.x→8.0 auto-upgrade', () => { expect(id).toBeTruthy() }) - it('blocks a read while migrating, then releases when the flag clears (poll path)', async () => { + it('blocks a graph read while the graph provider migrates, then releases when the flag clears (poll path)', async () => { + const anchor = await brain.add({ data: 'anchor', type: NounType.Concept }) let migrating = true brain.graphIndex.isMigrating = () => migrating - const p = brain.find({ type: NounType.Concept }) // a read → gated + const p = brain.related({ from: anchor }) // a GRAPH read → gated on the graph migration let resolved = false p.then(() => { resolved = true @@ -109,9 +115,12 @@ describe('Migration LOCK (#18) — coordinated 7.x→8.0 auto-upgrade', () => { migrationWaitTimeoutMs: 120 }) await shortBrain.init() - setMigrating(shortBrain, true) // never clears + const anchor = await shortBrain.add({ data: 'anchor', type: NounType.Concept }) + setMigrating(shortBrain, true) // graph lock never clears - await expect(shortBrain.find({ type: NounType.Concept })).rejects.toBeInstanceOf( + // A graph read needs the migrating family → it waits out the window and + // surfaces the retryable error rather than serving a half-built traversal. + await expect(shortBrain.related({ from: anchor })).rejects.toBeInstanceOf( MigrationInProgressError ) try {