mirror of
https://github.com/proffesor-for-testing/agentic-qe.git
synced 2026-09-19 08:45:47 +08:00
fix(persistence): fail closed on RVF richness ambiguity
This commit is contained in:
@@ -5,6 +5,7 @@
|
||||
| **Decision ID** | ADR-128 |
|
||||
| **Status** | Proposed — implementation candidate (dual-host review pending) |
|
||||
| **Date** | 2026-08-30 |
|
||||
| **Updated** | 2026-08-30 — adversarial review expanded the guard to all mirrored tables and fail-closed ambiguous inspection |
|
||||
| **Author** | AQE Core |
|
||||
| **Related** | ADR-072; issues #563, #574, #629 |
|
||||
|
||||
@@ -21,10 +22,10 @@ not distinguish a complete yet destructive export from a safe checkpoint.
|
||||
|
||||
## Decision
|
||||
|
||||
RVF promotion is fail-closed when the existing readable mirror contains more
|
||||
patterns than the candidate. The comparison occurs after the candidate is
|
||||
fully written and closed, immediately before atomic promotion. The existing
|
||||
store and sidecars remain untouched on refusal.
|
||||
RVF promotion is fail-closed when any mirrored table in the existing readable
|
||||
mirror contains more rows than the candidate. The comparison occurs after the
|
||||
candidate is fully written and closed, immediately before atomic promotion.
|
||||
The existing store and sidecars remain untouched on refusal.
|
||||
|
||||
The generated checkpoint no longer deletes the mirror or id map before export.
|
||||
An operator may explicitly acknowledge intentional data reduction with
|
||||
@@ -32,9 +33,10 @@ An operator may explicitly acknowledge intentional data reduction with
|
||||
flag. The error directs recovery through `brain import` before retrying.
|
||||
|
||||
An unreadable RVF remains replaceable so #563 corruption recovery is not
|
||||
regressed. Pattern count is the first conservative invariant because it is the
|
||||
loss demonstrated by #629; broader per-table monotonicity is a follow-up once
|
||||
table-specific deletion semantics are defined.
|
||||
regressed. If the RVF can still be opened and its kernel extracted but a
|
||||
complete table inventory cannot be established, promotion fails closed as
|
||||
ambiguous. Intentional deletion from any mirrored table requires the same
|
||||
explicit operator override as intentional pattern reduction.
|
||||
|
||||
## Consequences
|
||||
|
||||
@@ -45,12 +47,14 @@ table-specific deletion semantics are defined.
|
||||
- The guard does not automatically restore data. Automatic conflict resolution
|
||||
would mutate the primary and needs separate operator-approved recovery
|
||||
policy; refusal preserves both sides without guessing.
|
||||
- A valid mirror that loses only non-pattern tables is not yet detected. The
|
||||
candidate/existing manifest should later carry monotonic per-table counts.
|
||||
- Older readable mirrors without a complete table inventory require an
|
||||
explicit override; this favors preservation over an unprovable replacement.
|
||||
|
||||
## Verification
|
||||
|
||||
The #629 integration regression creates two disposable SQLite databases, exports
|
||||
25 patterns to RVF, attempts a five-pattern replacement, and proves the RVF and
|
||||
manifest remain byte-identical. It then proves the explicit override replaces
|
||||
the mirror. No project or production database is opened.
|
||||
The #629 integration regressions use disposable SQLite databases and cover:
|
||||
(1) 25 patterns versus five, (2) equal pattern counts but fewer captured
|
||||
experiences, (3) a readable mirror with ambiguous kernel metadata, and (4) the
|
||||
explicit override. The RVF and sidecars remain byte-identical on refusal. The
|
||||
#563 corrupt-store recovery regression remains green. No project or production
|
||||
database is opened.
|
||||
|
||||
@@ -45,9 +45,11 @@ export interface RvfBrainManifest {
|
||||
readonly patternCount: number;
|
||||
readonly embeddingCount: number;
|
||||
readonly qValueCount: number;
|
||||
readonly dreamInsightCount: number;
|
||||
readonly witnessChainLength: number;
|
||||
readonly totalRecords?: number;
|
||||
readonly dreamInsightCount: number;
|
||||
readonly witnessChainLength: number;
|
||||
readonly totalRecords?: number;
|
||||
/** Per-table row counts used by the recovery-mirror richness guard. */
|
||||
readonly tableCounts?: Readonly<Record<string, number>>;
|
||||
};
|
||||
readonly domains: readonly string[];
|
||||
readonly checksum: string;
|
||||
@@ -326,13 +328,16 @@ export function exportBrainToRvf(
|
||||
format: 'rvf',
|
||||
exportedAt: brainData.exportedAt,
|
||||
sourceDb: sourceDbLabel,
|
||||
stats: {
|
||||
patternCount: (allTableData['qe_patterns'] || []).length,
|
||||
embeddingCount,
|
||||
qValueCount: (allTableData['rl_q_values'] || []).length,
|
||||
dreamInsightCount: (allTableData['dream_insights'] || []).length,
|
||||
witnessChainLength: (allTableData['witness_chain'] || []).length,
|
||||
totalRecords,
|
||||
stats: {
|
||||
patternCount: (allTableData['qe_patterns'] || []).length,
|
||||
embeddingCount,
|
||||
qValueCount: (allTableData['rl_q_values'] || []).length,
|
||||
dreamInsightCount: (allTableData['dream_insights'] || []).length,
|
||||
witnessChainLength: (allTableData['witness_chain'] || []).length,
|
||||
totalRecords,
|
||||
tableCounts: Object.fromEntries(
|
||||
TABLE_CONFIGS.map(({ tableName }) => [tableName, allTableData[tableName]?.length ?? 0]),
|
||||
),
|
||||
},
|
||||
domains: brainData.domains,
|
||||
checksum,
|
||||
@@ -360,26 +365,58 @@ export function exportBrainToRvf(
|
||||
closed = true;
|
||||
|
||||
// Issue #629: a stage-2 mirror must not be destroyed when SQLite has just
|
||||
// been recreated from seed data. Compare the fully materialized candidate
|
||||
// with the still-published store immediately before promotion, closing the
|
||||
// TOCTOU window as far as this synchronous exporter can. A valid existing
|
||||
// mirror with more patterns wins unless an operator explicitly overrides.
|
||||
// been recreated from seed data. Compare every mirrored table in the fully
|
||||
// materialized candidate with the still-published store immediately before
|
||||
// promotion. Inspection is fail-closed when the RVF itself remains readable
|
||||
// but its kernel cannot yield a complete table inventory. A genuinely
|
||||
// unreadable/corrupt RVF remains replaceable to preserve #563 recovery.
|
||||
if (existsSync(outPath) && !options.allowOverwriteRicher) {
|
||||
let existing: RvfBrainManifest | undefined;
|
||||
try {
|
||||
const existing = brainInfoFromRvf(outPath);
|
||||
if (existing.stats.patternCount > manifest.stats.patternCount) {
|
||||
existing = brainInfoFromRvf(outPath);
|
||||
} catch (error) {
|
||||
if (hasReadableBrainKernel(outPath) || !isKnownUnusableRvfError(error)) {
|
||||
const detail = error instanceof Error ? error.message : String(error);
|
||||
throw new Error(
|
||||
`RVF_MIRROR_RICHER: refusing to replace ${existing.stats.patternCount} ` +
|
||||
`patterns with ${manifest.stats.patternCount}. Restore/import the mirror first, ` +
|
||||
`or repeat with --force-overwrite-richer after preserving a backup.`
|
||||
`RVF_MIRROR_INSPECTION_AMBIGUOUS: the existing mirror is readable but its ` +
|
||||
`richness could not be established (${detail}). Preserve/import it before retrying, ` +
|
||||
`or repeat with --force-overwrite-richer after preserving a backup.`,
|
||||
);
|
||||
}
|
||||
} catch (error) {
|
||||
if (error instanceof Error && error.message.startsWith('RVF_MIRROR_RICHER:')) {
|
||||
throw error;
|
||||
}
|
||||
|
||||
if (existing) {
|
||||
const existingCounts = existing.stats.tableCounts;
|
||||
const candidateCounts = manifest.stats.tableCounts!;
|
||||
const missing = TABLE_CONFIGS
|
||||
.map(({ tableName }) => tableName)
|
||||
.filter((tableName) => !existingCounts || !Number.isSafeInteger(existingCounts[tableName]));
|
||||
if (missing.length > 0) {
|
||||
throw new Error(
|
||||
`RVF_MIRROR_INSPECTION_AMBIGUOUS: existing mirror lacks trustworthy counts for ` +
|
||||
`${missing.join(', ')}. Preserve/import it before retrying, or repeat with ` +
|
||||
`--force-overwrite-richer after preserving a backup.`,
|
||||
);
|
||||
}
|
||||
|
||||
const richer = TABLE_CONFIGS
|
||||
.map(({ tableName }) => ({
|
||||
tableName,
|
||||
existing: existingCounts![tableName],
|
||||
candidate: candidateCounts[tableName],
|
||||
}))
|
||||
.filter(({ existing: existingCount, candidate }) => existingCount > candidate);
|
||||
if (richer.length > 0) {
|
||||
const differences = richer
|
||||
.map(({ tableName, existing: existingCount, candidate }) =>
|
||||
`${tableName} ${existingCount} -> ${candidate}`)
|
||||
.join(', ');
|
||||
throw new Error(
|
||||
`RVF_MIRROR_RICHER: refusing to replace richer mirrored tables: ${differences}. ` +
|
||||
`Restore/import the mirror first, or repeat with --force-overwrite-richer after ` +
|
||||
`preserving a backup.`,
|
||||
);
|
||||
}
|
||||
// An unreadable store is not a usable recovery source. Preserve the
|
||||
// existing #563 behavior that repairs it with a complete candidate.
|
||||
}
|
||||
}
|
||||
|
||||
@@ -410,7 +447,7 @@ export function exportBrainToRvf(
|
||||
|
||||
// --- Import ---
|
||||
|
||||
interface BrainKernelData {
|
||||
interface BrainKernelData {
|
||||
version: string;
|
||||
format: string;
|
||||
tables?: Record<string, Record<string, unknown>[]>;
|
||||
@@ -419,7 +456,34 @@ interface BrainKernelData {
|
||||
qValues?: unknown[];
|
||||
dreamInsights?: unknown[];
|
||||
witnessChain?: unknown[];
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Distinguish a corrupt/unopenable store (#563, replaceable) from a readable
|
||||
* store whose richness inspection failed (ambiguous, therefore protected).
|
||||
*/
|
||||
function hasReadableBrainKernel(rvfPath: string): boolean {
|
||||
let rvf: ReturnType<typeof openRvfStoreReadonly> | undefined;
|
||||
try {
|
||||
rvf = openRvfStoreReadonly(resolve(rvfPath));
|
||||
const kernel = rvf.extractKernel();
|
||||
return Boolean(kernel?.image);
|
||||
} catch {
|
||||
return false;
|
||||
} finally {
|
||||
try { rvf?.close(); } catch { /* best-effort probe cleanup */ }
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Only known structural corruption is replaceable without an operator
|
||||
* override. Permission, locking, I/O, and programmer errors are ambiguous and
|
||||
* must fail closed instead of being mistaken for disposable corruption.
|
||||
*/
|
||||
function isKnownUnusableRvfError(error: unknown): boolean {
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
return /ManifestNotFound|InvalidMagic|ChecksumMismatch|CorruptStore/i.test(message);
|
||||
}
|
||||
|
||||
/** Legacy field-to-table mapping for older RVF exports. */
|
||||
const LEGACY_FIELD_MAP: Record<string, string> = {
|
||||
@@ -595,11 +659,18 @@ export function brainInfoFromRvf(rvfPath: string): RvfBrainManifest {
|
||||
const status = rvf.status();
|
||||
const fileSize = statSync(filePath).size;
|
||||
|
||||
const t = brainData.tables;
|
||||
const t = brainData.tables;
|
||||
const patternCount = t ? (t['qe_patterns']?.length ?? 0) : (brainData.patterns?.length ?? 0);
|
||||
const qValueCount = t ? (t['rl_q_values']?.length ?? 0) : (brainData.qValues?.length ?? 0);
|
||||
const dreamInsightCount = t ? (t['dream_insights']?.length ?? 0) : (brainData.dreamInsights?.length ?? 0);
|
||||
const witnessChainLength = t ? (t['witness_chain']?.length ?? 0) : (brainData.witnessChain?.length ?? 0);
|
||||
const witnessChainLength = t ? (t['witness_chain']?.length ?? 0) : (brainData.witnessChain?.length ?? 0);
|
||||
const tableCounts = t && typeof t === 'object' && !Array.isArray(t)
|
||||
? Object.fromEntries(
|
||||
Object.entries(t)
|
||||
.filter((entry): entry is [string, Record<string, unknown>[]] => Array.isArray(entry[1]))
|
||||
.map(([tableName, rows]) => [tableName, rows.length]),
|
||||
)
|
||||
: undefined;
|
||||
|
||||
return {
|
||||
version: '3.0',
|
||||
@@ -609,9 +680,10 @@ export function brainInfoFromRvf(rvfPath: string): RvfBrainManifest {
|
||||
stats: {
|
||||
patternCount,
|
||||
embeddingCount: status.totalVectors,
|
||||
qValueCount,
|
||||
dreamInsightCount,
|
||||
witnessChainLength,
|
||||
qValueCount,
|
||||
dreamInsightCount,
|
||||
witnessChainLength,
|
||||
...(tableCounts ? { tableCounts } : {}),
|
||||
},
|
||||
domains: brainData.domains ?? [],
|
||||
checksum: sha256(brainJson),
|
||||
|
||||
@@ -27,7 +27,7 @@ import { spawn } from 'child_process';
|
||||
import { existsSync, readFileSync, writeFileSync, statSync, rmSync, mkdtempSync, mkdirSync, readdirSync } from 'fs';
|
||||
import { join } from 'path';
|
||||
|
||||
import { isRvfNativeAvailable, openRvfStore } from '../../src/integrations/ruvector/rvf-native-adapter.js';
|
||||
import { createRvfStore, isRvfNativeAvailable, openRvfStore } from '../../src/integrations/ruvector/rvf-native-adapter.js';
|
||||
import { exportBrainToRvf } from '../../src/integrations/ruvector/brain-rvf-exporter.js';
|
||||
import {
|
||||
quarantineUnusableStore,
|
||||
@@ -88,6 +88,15 @@ function seedDb(path: string, patternCount = 25): Database.Database {
|
||||
return db;
|
||||
}
|
||||
|
||||
function seedCapturedExperiences(db: Database.Database, count: number): void {
|
||||
const insert = db.prepare(`
|
||||
INSERT INTO captured_experiences
|
||||
(id, task, agent, domain, success, quality, duration_ms)
|
||||
VALUES (?, ?, 'qe-test-architect', 'test-generation', 1, 0.9, 1)
|
||||
`);
|
||||
for (let i = 0; i < count; i++) insert.run(`exp-${i}`, `experience ${i}`);
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
for (const d of dirs.splice(0)) {
|
||||
try { rmSync(d, { recursive: true, force: true }); } catch { /* best-effort */ }
|
||||
@@ -102,12 +111,14 @@ describeNative('RVF export atomicity under interruption (#563)', () => {
|
||||
exportBrainToRvf(richDb, { outputPath: rvfPath }, 'rich.db');
|
||||
richDb.close();
|
||||
const richBytes = readFileSync(rvfPath);
|
||||
const richIdMap = readFileSync(`${rvfPath}.idmap.json`, 'utf-8');
|
||||
const richManifest = readFileSync(`${rvfPath}.manifest.json`, 'utf-8');
|
||||
|
||||
const freshDb = seedDb(join(dir, 'fresh.db'), 5);
|
||||
expect(() => exportBrainToRvf(freshDb, { outputPath: rvfPath }, 'fresh.db'))
|
||||
.toThrow(/RVF_MIRROR_RICHER.*25 patterns with 5/);
|
||||
.toThrow(/RVF_MIRROR_RICHER.*qe_patterns 25 -> 5/);
|
||||
expect(readFileSync(rvfPath).equals(richBytes)).toBe(true);
|
||||
expect(readFileSync(`${rvfPath}.idmap.json`, 'utf-8')).toBe(richIdMap);
|
||||
expect(readFileSync(`${rvfPath}.manifest.json`, 'utf-8')).toBe(richManifest);
|
||||
|
||||
const forced = exportBrainToRvf(
|
||||
@@ -119,6 +130,53 @@ describeNative('RVF export atomicity under interruption (#563)', () => {
|
||||
freshDb.close();
|
||||
}, 30_000);
|
||||
|
||||
it('refuses equal-pattern replacement when another mirrored table is richer (#629)', () => {
|
||||
const dir = workDir();
|
||||
const rvfPath = join(dir, 'brain.rvf');
|
||||
const richDb = seedDb(join(dir, 'rich-other-table.db'), 5);
|
||||
seedCapturedExperiences(richDb, 7);
|
||||
exportBrainToRvf(richDb, { outputPath: rvfPath }, 'rich-other-table.db');
|
||||
richDb.close();
|
||||
const richBytes = readFileSync(rvfPath);
|
||||
const richIdMap = readFileSync(`${rvfPath}.idmap.json`, 'utf-8');
|
||||
const richManifest = readFileSync(`${rvfPath}.manifest.json`, 'utf-8');
|
||||
|
||||
const poorerDb = seedDb(join(dir, 'poorer-other-table.db'), 5);
|
||||
expect(() => exportBrainToRvf(poorerDb, { outputPath: rvfPath }, 'poorer-other-table.db'))
|
||||
.toThrow(/RVF_MIRROR_RICHER.*captured_experiences.*7.*0/);
|
||||
expect(readFileSync(rvfPath).equals(richBytes)).toBe(true);
|
||||
expect(readFileSync(`${rvfPath}.idmap.json`, 'utf-8')).toBe(richIdMap);
|
||||
expect(readFileSync(`${rvfPath}.manifest.json`, 'utf-8')).toBe(richManifest);
|
||||
poorerDb.close();
|
||||
}, 30_000);
|
||||
|
||||
it('fails closed when a readable mirror cannot be inspected unambiguously (#629)', () => {
|
||||
const dir = workDir();
|
||||
const rvfPath = join(dir, 'brain.rvf');
|
||||
const ambiguous = createRvfStore(rvfPath, 384);
|
||||
ambiguous.embedKernel(Buffer.from('{"version":"3.0","format":"rvf","tables":null}'));
|
||||
ambiguous.close();
|
||||
const originalBytes = readFileSync(rvfPath);
|
||||
|
||||
const candidateDb = seedDb(join(dir, 'candidate.db'), 5);
|
||||
expect(() => exportBrainToRvf(candidateDb, { outputPath: rvfPath }, 'candidate.db'))
|
||||
.toThrow(/RVF_MIRROR_INSPECTION_AMBIGUOUS/);
|
||||
expect(readFileSync(rvfPath).equals(originalBytes)).toBe(true);
|
||||
candidateDb.close();
|
||||
}, 30_000);
|
||||
|
||||
it('still replaces a mirror with known structural corruption (#563)', () => {
|
||||
const dir = workDir();
|
||||
const rvfPath = join(dir, 'brain.rvf');
|
||||
writeFileSync(rvfPath, Buffer.concat([Buffer.from('SFVR'), Buffer.alloc(158)]));
|
||||
|
||||
const candidateDb = seedDb(join(dir, 'recovery-candidate.db'), 5);
|
||||
const manifest = exportBrainToRvf(candidateDb, { outputPath: rvfPath }, 'recovery-candidate.db');
|
||||
expect(manifest.stats.patternCount).toBe(5);
|
||||
expect(() => openRvfStore(rvfPath).close()).not.toThrow();
|
||||
candidateDb.close();
|
||||
}, 30_000);
|
||||
|
||||
it('leaves the previous good store intact when the export is SIGKILLed mid-write', async () => {
|
||||
const dir = workDir();
|
||||
const dbPath = join(dir, 'memory.db');
|
||||
|
||||
Reference in New Issue
Block a user