fix(8.0): close GA-blocking correctness gaps from the readiness audit
- Version-coupling cold-init: getBrainyVersion() returned a stale build-time
default ('3.14.0') on the FIRST synchronous call — the one loadPlugins() makes
to build context.version — because the package.json read was async. A native
provider declaring a realistic `>=8.0.0` range was therefore rejected on cold
init. version.ts now reads package.json synchronously (8.0 targets Node-like
runtimes only); added a cold-init coupling regression with a `^8.0.0` plugin.
- No-silent-failures on the highest-fan-in read path: getNouns()/getVerbs()
converted a storage read failure into a success-shaped empty page, which the
cold-start rebuild then read as "store empty" and skipped the rebuild — booting
a permanently-empty index with no signal (the same silent-failure class as the
phantom bug). Both now re-throw a named BrainyError; the rebuild path re-throws
read failures (fail loud) and records a queryable degraded state, surfaced via
checkHealth(), for non-fatal rebuild hiccups.
- LSM SSTable leak: graph compaction wrote a merged SSTable but only dropped the
old ones from the manifest, orphaning their payloads forever ("In production
we'd add a cleanup mechanism"). Added StorageAdapter.deleteMetadata() and now
reclaim each compacted-away SSTable.
- Documented the two headline methods add() and find() (the only undocumented
public methods on the class).
Full gate green: build, unit 1512, integration 607.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
40d2cd5419
commit
47e8031124
7 changed files with 291 additions and 133 deletions
|
|
@ -30,6 +30,7 @@ import { getShardId } from './sharding.js'
|
|||
import { BlobStorage, type BlobStoreAdapter } from './blobStorage.js'
|
||||
import { unwrapBinaryData } from './binaryDataCodec.js'
|
||||
import { prodLog } from '../utils/logger.js'
|
||||
import { BrainyError } from '../errors/brainyError.js'
|
||||
import { MetadataWriteBuffer } from '../utils/metadataWriteBuffer.js'
|
||||
import {
|
||||
splitNounMetadataRecord,
|
||||
|
|
@ -1568,23 +1569,24 @@ export abstract class BaseStorage extends BaseStorageAdapter {
|
|||
}
|
||||
}
|
||||
|
||||
// Storage adapter does not support pagination
|
||||
prodLog.error(
|
||||
'Storage adapter does not support pagination. The deprecated getAllNouns_internal() method has been removed. Please implement getNounsWithPagination() in your storage adapter.'
|
||||
// Storage adapter does not support pagination. This is a hard
|
||||
// misconfiguration — find()/rebuild/aggregation all read through this path,
|
||||
// so returning an empty page would silently present a misconfigured adapter
|
||||
// as an empty database. Fail loud instead.
|
||||
throw BrainyError.storage(
|
||||
'Storage adapter does not implement getNounsWithPagination(). The deprecated getAllNouns_internal() fallback has been removed; implement pagination in your storage adapter.'
|
||||
)
|
||||
|
||||
return {
|
||||
items: [],
|
||||
totalCount: 0,
|
||||
hasMore: false
|
||||
}
|
||||
} catch (error) {
|
||||
prodLog.error('Error getting nouns with pagination:', error)
|
||||
return {
|
||||
items: [],
|
||||
totalCount: 0,
|
||||
hasMore: false
|
||||
}
|
||||
// Never convert a genuine read failure into a success-shaped empty page:
|
||||
// this is the highest-fan-in read path (find() fallback, cold-start rebuild
|
||||
// gating, aggregation backfill, getNounsByNounType), so a swallowed error
|
||||
// would propagate as "zero rows" everywhere — indistinguishable from a truly
|
||||
// empty store, and the exact silent-failure class the 8.0 contract forbids.
|
||||
if (error instanceof BrainyError) throw error
|
||||
throw BrainyError.storage(
|
||||
'getNouns pagination read failed',
|
||||
error instanceof Error ? error : undefined
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -2415,12 +2417,14 @@ export abstract class BaseStorage extends BaseStorageAdapter {
|
|||
: undefined
|
||||
}
|
||||
} catch (error) {
|
||||
prodLog.error('Error getting verbs with pagination:', error)
|
||||
return {
|
||||
items: [],
|
||||
totalCount: 0,
|
||||
hasMore: false
|
||||
}
|
||||
// Same no-silent-failure contract as getNouns: a genuine read failure must
|
||||
// surface as a named, catchable error, not a success-shaped empty page that
|
||||
// callers read as "this graph has no verbs."
|
||||
if (error instanceof BrainyError) throw error
|
||||
throw BrainyError.storage(
|
||||
'getVerbs pagination read failed',
|
||||
error instanceof Error ? error : undefined
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -2576,6 +2580,26 @@ export abstract class BaseStorage extends BaseStorageAdapter {
|
|||
return null
|
||||
}
|
||||
|
||||
/**
|
||||
* Delete a system/metadata object previously written with {@link saveMetadata}.
|
||||
* The exact inverse of save/get: routes through `analyzeKey(id, 'system')` and
|
||||
* removes the canonical object (plus the legacy flat path that `getMetadata`
|
||||
* also reads, so a sharded key leaves nothing behind). Lets keyed-payload
|
||||
* owners — e.g. the LSM graph store reclaiming its compacted-away SSTables —
|
||||
* free storage instead of orphaning it. Idempotent: deleting a missing path is
|
||||
* a no-op.
|
||||
*/
|
||||
public async deleteMetadata(id: string): Promise<void> {
|
||||
await this.ensureInitialized()
|
||||
const keyInfo = this.analyzeKey(id, 'system')
|
||||
await this.deleteCanonicalObject(keyInfo.fullPath)
|
||||
// Mirror getMetadata's legacy fallback so an older flat-path payload for a
|
||||
// sharded key is also reclaimed.
|
||||
if (keyInfo.shardId !== null) {
|
||||
await this.deleteCanonicalObject(`${SYSTEM_DIR}/${id}.json`)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Save noun metadata to storage (now typed)
|
||||
* Routes to correct sharded location based on UUID
|
||||
|
|
@ -3175,15 +3199,11 @@ export abstract class BaseStorage extends BaseStorageAdapter {
|
|||
await this.ensureInitialized()
|
||||
|
||||
// Direct O(1) lookup with ID-first paths - no type search needed!
|
||||
// Symmetric with getNounMetadata: readCanonicalObject already returns null for
|
||||
// a genuine not-found, so a real storage fault (permission/corruption/IO) must
|
||||
// propagate rather than be masked as "this verb has no metadata".
|
||||
const path = getVerbMetadataPath(id)
|
||||
|
||||
try {
|
||||
const metadata = await this.readCanonicalObject(path)
|
||||
return metadata || null
|
||||
} catch (error) {
|
||||
// Entity not found
|
||||
return null
|
||||
}
|
||||
return this.readCanonicalObject(path)
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue