fix(transact): metadata-index ops take their JSON-safe view at the crossing, not at construction
transact()'s delete legs (direct unrelate and the noun-remove cascade) hand the SAME verb object to the graph-retraction op and the metadata-retraction op. The metadata leg sanitized at PLAN time, when the verb was still clean, so the wrap returned the same reference — then the graph op's execute-time endpoint resolution (deliberately deferred for same-batch forward refs) mirrored BigInt sourceInt/targetInt onto the shared object, and the metadata op crossed the seam with them. A strict provider rightly refuses that crossing, so every transact-wrapped edge delete aborted; direct unrelate() resolves ints at build time, before its sanitize, which is why no existing gate saw it. The JSON-safe view now lives in a shared leaf (utils/jsonSafeIndexMetadata) and is applied INSIDE AddToMetadataIndexOperation and RemoveFromMetadataIndexOperation at execute and rollback time — the one place no plan-vs-execute ordering can bypass. Pins: the fleet repro, the cascade shape, a mixed batch, and unit pins that mutate the entity after construction against a strict seam (5 red before, 5 green after).
This commit is contained in:
parent
0f0022b1c9
commit
73500e7d10
4 changed files with 263 additions and 27 deletions
|
|
@ -15,6 +15,7 @@ import { JsHnswVectorIndex } from './hnsw/hnswIndex.js'
|
|||
import { createStorage, resolveFilesystemRoot } from './storage/storageFactory.js'
|
||||
import type { StorageOptions } from './storage/storageFactory.js'
|
||||
import { rebuildCounts } from './utils/rebuildCounts.js'
|
||||
import { jsonSafeIndexMetadata } from './utils/jsonSafeIndexMetadata.js'
|
||||
import type { MetadataWriteBuffer } from './utils/metadataWriteBuffer.js'
|
||||
import { BaseStorage } from './storage/baseStorage.js'
|
||||
import {
|
||||
|
|
@ -4203,32 +4204,19 @@ export class Brainy<T = any> implements BrainyInterface<T> {
|
|||
*/
|
||||
/**
|
||||
* @description A JSON-safe view of a record bound for the metadata-index
|
||||
* crossing. The seam's metadata is JSON-safe BY CONTRACT (a native provider
|
||||
* serializes it; u64 ints as Number corrupt above 2^53) — but
|
||||
* {@link resolveVerbEndpointInts} MIRRORS the resolved endpoint ints onto
|
||||
* the verb object itself as BigInt (`verb.sourceInt`/`targetInt`), so a
|
||||
* verb object reused as index metadata carried BigInts into
|
||||
* JSON.stringify, which throws, aborting the whole transaction (found by
|
||||
* the first joint pair gate). Endpoint ints ride their OWN op params on the
|
||||
* graph legs — the metadata crossing drops every BigInt-valued top-level
|
||||
* key instead of guessing at a lossy numeric encoding.
|
||||
* crossing — delegates to the shared {@link jsonSafeIndexMetadata} leaf,
|
||||
* which the metadata-index transaction operations ALSO apply at execute
|
||||
* and rollback time. This plan-time wrap alone proved insufficient: it
|
||||
* returns the same reference when the record is clean, and `transact()`'s
|
||||
* delete legs share that reference with a graph-retraction op whose
|
||||
* execute-time endpoint resolution mirrors BigInt ints onto it (the full
|
||||
* aliasing story lives on the leaf module's doc).
|
||||
* @param metadata - The candidate index-metadata record.
|
||||
* @returns The same object when already JSON-safe, else a shallow copy
|
||||
* without the BigInt-valued keys.
|
||||
*/
|
||||
private static jsonSafeIndexMetadata(metadata: unknown): unknown {
|
||||
if (metadata === null || typeof metadata !== 'object') return metadata
|
||||
const rec = metadata as Record<string, unknown>
|
||||
let hasBigint = false
|
||||
for (const k in rec) {
|
||||
if (typeof rec[k] === 'bigint') { hasBigint = true; break }
|
||||
}
|
||||
if (!hasBigint) return metadata
|
||||
const out: Record<string, unknown> = {}
|
||||
for (const k in rec) {
|
||||
if (typeof rec[k] !== 'bigint') out[k] = rec[k]
|
||||
}
|
||||
return out
|
||||
return jsonSafeIndexMetadata(metadata)
|
||||
}
|
||||
|
||||
private metadataIndexRetractionOp(
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ import type { MetadataIndexManager } from '../../utils/metadataIndex.js'
|
|||
import type { GraphVerb } from '../../coreTypes.js'
|
||||
import type { Operation, RollbackAction } from '../types.js'
|
||||
import { isZeroNormVector } from '../../utils/distance.js'
|
||||
import { jsonSafeIndexMetadata } from '../../utils/jsonSafeIndexMetadata.js'
|
||||
import { prodLog } from '../../utils/logger.js'
|
||||
|
||||
/**
|
||||
|
|
@ -390,13 +391,21 @@ export class AddToMetadataIndexOperation implements Operation {
|
|||
// rollback so add + undo reference the same watermark.
|
||||
const generation = this.generationFn?.()
|
||||
|
||||
// Add to metadata index (skipFlush=true for transaction atomicity)
|
||||
await this.index.addToIndex(this.id, this.entity, true, false, generation)
|
||||
// The JSON-safe view is taken HERE, per crossing, never at construction:
|
||||
// the entity reference this op holds can be mutated between plan and
|
||||
// execute (a graph op's execute-time endpoint-int resolution mirrors
|
||||
// BigInts onto a shared verb object) — see jsonSafeIndexMetadata's
|
||||
// module doc.
|
||||
await this.index.addToIndex(
|
||||
this.id, jsonSafeIndexMetadata(this.entity), true, false, generation
|
||||
)
|
||||
|
||||
// Return rollback action
|
||||
return async () => {
|
||||
// Remove from metadata index
|
||||
await this.index.removeFromIndex(this.id, this.entity, generation)
|
||||
await this.index.removeFromIndex(
|
||||
this.id, jsonSafeIndexMetadata(this.entity), generation
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -432,13 +441,21 @@ export class RemoveFromMetadataIndexOperation implements Operation {
|
|||
// Resolve the removal generation once; reuse it for the rollback re-add.
|
||||
const generation = this.generationFn?.()
|
||||
|
||||
// Remove from metadata index
|
||||
await this.index.removeFromIndex(this.id, this.entity, generation)
|
||||
// Sanitized per crossing, never at construction — transact()'s delete
|
||||
// legs hand this op the SAME verb object the graph-retraction op's
|
||||
// execute-time endpoint resolution mutates (BigInt sourceInt/targetInt),
|
||||
// so a plan-time view aliases the pollution. See jsonSafeIndexMetadata's
|
||||
// module doc.
|
||||
await this.index.removeFromIndex(
|
||||
this.id, jsonSafeIndexMetadata(this.entity), generation
|
||||
)
|
||||
|
||||
// Return rollback action
|
||||
return async () => {
|
||||
// Re-add with original metadata (skipFlush=true)
|
||||
await this.index.addToIndex(this.id, this.entity, true, false, generation)
|
||||
await this.index.addToIndex(
|
||||
this.id, jsonSafeIndexMetadata(this.entity), true, false, generation
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
47
src/utils/jsonSafeIndexMetadata.ts
Normal file
47
src/utils/jsonSafeIndexMetadata.ts
Normal file
|
|
@ -0,0 +1,47 @@
|
|||
/**
|
||||
* @module utils/jsonSafeIndexMetadata
|
||||
* @description The metadata-index crossing's JSON-safety law, as a leaf
|
||||
* function both the coordinator and the transaction operations share.
|
||||
*
|
||||
* The seam's metadata is JSON-safe BY CONTRACT (a native provider serializes
|
||||
* it; u64 ints as Number corrupt above 2^53) — but `resolveVerbEndpointInts`
|
||||
* MIRRORS the resolved endpoint ints onto the verb object itself as BigInt
|
||||
* (`verb.sourceInt`/`targetInt`), so a verb object reused as index metadata
|
||||
* carries BigInts into JSON.stringify, which throws, aborting the whole
|
||||
* transaction. Endpoint ints ride their OWN op params on the graph legs — the
|
||||
* metadata crossing drops every BigInt-valued top-level key instead of
|
||||
* guessing at a lossy numeric encoding.
|
||||
*
|
||||
* WHY THIS IS A LEAF MODULE, ENFORCED AT THE CROSSING: sanitizing only at
|
||||
* operation-construction time is not enough. `transact()`'s delete legs pass
|
||||
* the SAME verb object to both the graph-retraction op (whose endpoint-int
|
||||
* thunk deliberately resolves at EXECUTE time, for same-batch forward refs)
|
||||
* and the metadata-retraction op. At plan time the verb is still clean, so a
|
||||
* plan-time sanitize returns the same reference — then the graph op executes
|
||||
* first, mirrors the BigInt ints onto the shared object, and the metadata op
|
||||
* crosses the seam with them (found by the first fleet adoption of the native
|
||||
* pair: every transact-wrapped edge delete aborted). The crossing itself is
|
||||
* the only place ordering cannot bypass.
|
||||
*/
|
||||
|
||||
/**
|
||||
* A JSON-safe view of a record bound for the metadata-index crossing.
|
||||
*
|
||||
* @param metadata - The candidate index-metadata record.
|
||||
* @returns The same object when already JSON-safe, else a shallow copy
|
||||
* without the BigInt-valued keys.
|
||||
*/
|
||||
export function jsonSafeIndexMetadata(metadata: unknown): unknown {
|
||||
if (metadata === null || typeof metadata !== 'object') return metadata
|
||||
const rec = metadata as Record<string, unknown>
|
||||
let hasBigint = false
|
||||
for (const k in rec) {
|
||||
if (typeof rec[k] === 'bigint') { hasBigint = true; break }
|
||||
}
|
||||
if (!hasBigint) return metadata
|
||||
const out: Record<string, unknown> = {}
|
||||
for (const k in rec) {
|
||||
if (typeof rec[k] !== 'bigint') out[k] = rec[k]
|
||||
}
|
||||
return out
|
||||
}
|
||||
184
tests/integration/transact-edge-delete-bigint-aliasing.test.ts
Normal file
184
tests/integration/transact-edge-delete-bigint-aliasing.test.ts
Normal file
|
|
@ -0,0 +1,184 @@
|
|||
/**
|
||||
* @module tests/integration/transact-edge-delete-bigint-aliasing
|
||||
* @description Regression for a fleet-adoption blocker: ANY edge delete
|
||||
* inside `transact()` — a direct unrelate or a noun-remove's cascade —
|
||||
* aborted with the metadata seam's BigInt JSON-guard error on a strict
|
||||
* (native) metadata provider.
|
||||
*
|
||||
* The aliasing chain: `planTxUnrelate`/the remove-cascade pass the SAME verb
|
||||
* object to the graph-retraction op and the metadata-retraction op. The
|
||||
* metadata leg's JSON-safe wrap ran at PLAN time, when the verb was still
|
||||
* clean — so it returned the same reference. At EXECUTE time the graph op
|
||||
* runs first and `resolveVerbEndpointInts` mirrors BigInt
|
||||
* `sourceInt`/`targetInt` onto the shared object (deliberately deferred for
|
||||
* same-batch forward refs — see transact-forward-ref-graph.test.ts); the
|
||||
* metadata op then crossed the seam with the polluted object. Direct
|
||||
* `unrelate()` resolves ints at BUILD time, before its sanitize, which is why
|
||||
* only the transact() shapes ever hit it.
|
||||
*
|
||||
* Fix under pin: the JSON-safe view is taken AT THE CROSSING — inside the
|
||||
* metadata-index operations' execute/rollback — so no plan-vs-execute
|
||||
* ordering can bypass it. The JS baseline index tolerates BigInts (it would
|
||||
* mask the bug), so these pins SPY on the seam and assert what actually
|
||||
* crossed, exactly as a strict native provider would judge it.
|
||||
*/
|
||||
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/brainy.js'
|
||||
import { NounType, VerbType } from '../../src/types/graphTypes.js'
|
||||
import {
|
||||
AddToMetadataIndexOperation,
|
||||
RemoveFromMetadataIndexOperation
|
||||
} from '../../src/transaction/operations/index.js'
|
||||
|
||||
let seq = 0
|
||||
const freshId = (): string =>
|
||||
`00000000-0000-4000-8000-${(++seq).toString(16).padStart(12, '0')}`
|
||||
|
||||
/** Top-level BigInt-valued keys of a candidate seam crossing (the guard's law). */
|
||||
const bigintKeys = (metadata: unknown): string[] => {
|
||||
if (metadata === null || typeof metadata !== 'object') return []
|
||||
return Object.entries(metadata as Record<string, unknown>)
|
||||
.filter(([, v]) => typeof v === 'bigint')
|
||||
.map(([k]) => k)
|
||||
}
|
||||
|
||||
describe('transact() edge deletes never carry BigInt across the metadata seam', () => {
|
||||
let dir: string
|
||||
let brain: any
|
||||
let crossings: Array<{ door: string; id: string; keys: string[] }>
|
||||
|
||||
beforeEach(async () => {
|
||||
process.env.BRAINY_DETERMINISTIC_EMBEDDINGS = 'true'
|
||||
dir = fs.mkdtempSync(path.join(os.tmpdir(), 'brainy-tx-bigint-'))
|
||||
brain = new Brainy({
|
||||
requireSubtype: false,
|
||||
storage: { type: 'filesystem', path: dir },
|
||||
dimensions: 384,
|
||||
silent: true
|
||||
})
|
||||
await brain.init()
|
||||
|
||||
// Spy on the seam the way a strict native provider judges it: record the
|
||||
// BigInt-valued top-level keys of every metadata argument that crosses.
|
||||
// The JS baseline index tolerates BigInts, so without this the baseline
|
||||
// run would green a shape the native pair aborts on.
|
||||
crossings = []
|
||||
const index = brain.metadataIndex
|
||||
for (const door of ['addToIndex', 'removeFromIndex'] as const) {
|
||||
const real = index[door].bind(index)
|
||||
index[door] = (id: string, metadata: unknown, ...rest: unknown[]) => {
|
||||
crossings.push({ door, id, keys: bigintKeys(metadata) })
|
||||
return real(id, metadata, ...rest)
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
await brain.close()
|
||||
fs.rmSync(dir, { recursive: true, force: true })
|
||||
})
|
||||
|
||||
it('CASE 1 (the fleet repro): relate, then transact([{op: unrelate}])', async () => {
|
||||
const a = await brain.add({ id: freshId(), data: 'a', type: NounType.Thing })
|
||||
const b = await brain.add({ id: freshId(), data: 'b', type: NounType.Thing })
|
||||
const verbId = await brain.relate({ from: a, to: b, type: VerbType.RelatedTo })
|
||||
|
||||
crossings.length = 0
|
||||
await brain.transact([{ op: 'unrelate', id: verbId }])
|
||||
|
||||
const polluted = crossings.filter((c) => c.keys.length > 0)
|
||||
expect(polluted).toEqual([])
|
||||
expect(await brain.storage.getVerb(verbId)).toBeFalsy()
|
||||
})
|
||||
|
||||
it('CASE 2 (the cascade shape): transact([{op: remove}]) cascading edge deletes', async () => {
|
||||
const a = await brain.add({ id: freshId(), data: 'a', type: NounType.Thing })
|
||||
const b = await brain.add({ id: freshId(), data: 'b', type: NounType.Thing })
|
||||
const c = await brain.add({ id: freshId(), data: 'c', type: NounType.Thing })
|
||||
const ab = await brain.relate({ from: a, to: b, type: VerbType.RelatedTo })
|
||||
const ca = await brain.relate({ from: c, to: a, type: VerbType.RelatedTo })
|
||||
|
||||
crossings.length = 0
|
||||
await brain.transact([{ op: 'remove', id: a }])
|
||||
|
||||
const polluted = crossings.filter((c2) => c2.keys.length > 0)
|
||||
expect(polluted).toEqual([])
|
||||
expect(await brain.get(a)).toBeFalsy()
|
||||
expect(await brain.storage.getVerb(ab)).toBeFalsy()
|
||||
expect(await brain.storage.getVerb(ca)).toBeFalsy()
|
||||
})
|
||||
|
||||
it('CASE 3 (one batch, both legs): adds + relate + unrelate of a pre-existing edge', async () => {
|
||||
const a = await brain.add({ id: freshId(), data: 'a', type: NounType.Thing })
|
||||
const b = await brain.add({ id: freshId(), data: 'b', type: NounType.Thing })
|
||||
const old = await brain.relate({ from: a, to: b, type: VerbType.RelatedTo })
|
||||
|
||||
const x = freshId()
|
||||
crossings.length = 0
|
||||
await brain.transact([
|
||||
{ op: 'add', id: x, data: 'x', type: NounType.Thing },
|
||||
{ op: 'relate', from: a, to: x, type: VerbType.RelatedTo },
|
||||
{ op: 'unrelate', id: old }
|
||||
])
|
||||
|
||||
const polluted = crossings.filter((c) => c.keys.length > 0)
|
||||
expect(polluted).toEqual([])
|
||||
expect(await brain.storage.getVerb(old)).toBeFalsy()
|
||||
const edges = await brain.related({ from: a })
|
||||
expect(edges.length).toBe(1)
|
||||
expect(edges[0].id).not.toBe(old)
|
||||
})
|
||||
})
|
||||
|
||||
describe('the metadata-index operations sanitize at the crossing, not at construction', () => {
|
||||
/** A strict seam: refuses BigInts exactly as the native provider does. */
|
||||
const strictIndex = () => {
|
||||
const seen: Array<{ door: string; keys: string[] }> = []
|
||||
const judge = (door: string, metadata: unknown) => {
|
||||
const keys = bigintKeys(metadata)
|
||||
seen.push({ door, keys })
|
||||
if (keys.length > 0) {
|
||||
throw new Error(
|
||||
`${door}: the metadata object violates the provider seam's JSON ` +
|
||||
`contract — BigInt at ${keys.join(', ')}.`
|
||||
)
|
||||
}
|
||||
}
|
||||
return {
|
||||
seen,
|
||||
addToIndex: async (_id: string, metadata: unknown) => judge('addToIndex', metadata),
|
||||
removeFromIndex: async (_id: string, metadata: unknown) => judge('removeFromIndex', metadata)
|
||||
}
|
||||
}
|
||||
|
||||
it('RemoveFromMetadataIndexOperation: entity mutated AFTER construction still crosses clean', async () => {
|
||||
const index = strictIndex()
|
||||
const verb: Record<string, unknown> = { id: 'v1', sourceId: 'a', targetId: 'b' }
|
||||
const op = new RemoveFromMetadataIndexOperation(index as any, 'v1', verb, () => 7n)
|
||||
|
||||
// The graph leg's execute-time endpoint resolution, simulated: the shared
|
||||
// object is polluted between plan and execute.
|
||||
verb.sourceInt = 800_000n
|
||||
verb.targetInt = 800_001n
|
||||
|
||||
const rollback = await op.execute()
|
||||
await rollback()
|
||||
expect(index.seen.map((s) => s.keys)).toEqual([[], []])
|
||||
})
|
||||
|
||||
it('AddToMetadataIndexOperation: same law on the add leg and its rollback', async () => {
|
||||
const index = strictIndex()
|
||||
const verb: Record<string, unknown> = { id: 'v2', sourceId: 'a', targetId: 'b' }
|
||||
const op = new AddToMetadataIndexOperation(index as any, 'v2', verb, () => 7n)
|
||||
|
||||
verb.sourceInt = 800_000n
|
||||
verb.targetInt = 800_001n
|
||||
|
||||
const rollback = await op.execute()
|
||||
await rollback()
|
||||
expect(index.seen.map((s) => s.keys)).toEqual([[], []])
|
||||
})
|
||||
})
|
||||
Loading…
Add table
Add a link
Reference in a new issue