Skip to content

Commit 8b34fe3

Browse files
fix(ai): flag empty-parity backups as not a healthy pass (#14048) (#14049)
* fix(ai): flag empty-parity backups as not a healthy pass (#14048) verifyBundleIntegrity reported status:'pass' whenever bundleCount === sourceCount, including when both are zero. A backup of a gutted store — the #13999 signature, where the source was already emptied so the export returns 0 and the bundle is 0 — therefore passed as healthy, indistinguishable from a real backup. An all-empty 'passing' backup is a false recovery source, the exact trust gap the backup substrate must close. Distinguish status:'empty' (source === bundle === 0) from a healthy 'pass' (N === N, N > 0). runBackup surfaces empty subsystems with a loud, non-fatal warning (a fresh environment legitimately backs up empty subsystems) and the 'empty' status is carried in bundle-meta.integrity so a downstream canary/alert can escalate. (block-alignment --fix also corrected pre-existing import over-padding in the touched file.) * fix(ai): document empty integrity status + prove runBackup propagation (#14048) Addresses the #14049 review: - Contract doc: verifyBundleIntegrity's JSDoc (body + @returns) now documents the 'empty' status (zero-zero parity, non-fatal, not a usable recovery source) alongside pass/fail/skipped — the previously-stale return contract. - Propagation proof: a new test drives runBackup with an empty MC subsystem and asserts status:'empty' is persisted into bundle-meta.integrity AND a non-fatal warning fires — proving the canary/alert handoff, not just the helper branch. Contract impact verified: no current consumer switches on bundle-meta.integrity[].status (restore.mjs runs its own validation + reads topology/version; HealthService reads only completedAt/timestamp). 'empty' is additive; the intended consumer is the future canary.
1 parent aaf64b7 commit 8b34fe3

2 files changed

Lines changed: 111 additions & 27 deletions

File tree

ai/scripts/maintenance/backup.mjs

Lines changed: 56 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,19 @@
1-
import {execFile} from 'child_process';
2-
import fs from 'fs-extra';
3-
import path from 'path';
4-
import {promisify} from 'util';
5-
import {fileURLToPath} from 'url';
1+
import {execFile} from 'child_process';
2+
import fs from 'fs-extra';
3+
import path from 'path';
4+
import {promisify} from 'util';
5+
import {fileURLToPath} from 'url';
66

77
// Neo namespace bootstrap (entry-point invariant) for the operator-runnable backup driver.
88
// `Neo` + `core/_export` populate
99
// `globalThis.Neo`; `InstanceManager` binds Neo.find/findFirst/get aliases +
1010
// consumes pre-singleton `Neo.idMap`.
11-
import Neo from '../../../src/Neo.mjs';
12-
import * as core from '../../../src/core/_export.mjs';
13-
import InstanceManager from '../../../src/manager/Instance.mjs';
14-
import AiConfig from '../../config.mjs';
15-
import kbConfig from '../../mcp/server/knowledge-base/config.mjs';
16-
import mcConfig from '../../mcp/server/memory-core/config.mjs';
11+
import Neo from '../../../src/Neo.mjs';
12+
import * as core from '../../../src/core/_export.mjs';
13+
import InstanceManager from '../../../src/manager/Instance.mjs';
14+
import AiConfig from '../../config.mjs';
15+
import kbConfig from '../../mcp/server/knowledge-base/config.mjs';
16+
import mcConfig from '../../mcp/server/memory-core/config.mjs';
1717

1818
import {
1919
KB_DatabaseService,
@@ -157,7 +157,7 @@ export async function runBackup({
157157
sentToCullSourceFile = DEFAULT_SENT_TO_CULL_FILE,
158158
logger = console
159159
} = {}) {
160-
const timestamp = new Date().toISOString().replace(/:/g, '-');
160+
const timestamp = new Date().toISOString().replace(/:/g, '-');
161161
const resolvedRoot = bundleRoot ?? path.join(DEFAULT_BACKUP_ROOT, `backup-${timestamp}`);
162162

163163
const layout = {
@@ -214,7 +214,7 @@ export async function runBackup({
214214
await cleanOldBackups(DEFAULT_BACKUP_ROOT, logger, resolveBackupRetention());
215215

216216
logger.log('Verifying bundle integrity (row-count parity)...');
217-
const integrity = await verifyBundleIntegrity(layout, subsystems);
217+
const integrity = await verifyBundleIntegrity(layout, subsystems);
218218
const failedChecks = integrity.filter(check => check.status === 'fail');
219219
if (failedChecks.length > 0) {
220220
throw new Error(
@@ -223,10 +223,23 @@ export async function runBackup({
223223
);
224224
}
225225

226+
const emptyChecks = integrity.filter(check => check.status === 'empty');
227+
if (emptyChecks.length > 0) {
228+
// Non-fatal (a fresh environment legitimately backs up empty subsystems), but loud: a zero-row
229+
// export from a normally-populated deployment is the gutted-store signature, and a backup of an
230+
// empty store is a false recovery source. The 'empty' status is carried in bundle-meta.integrity
231+
// for a downstream canary/alert to escalate on.
232+
logger.warn?.(
233+
`[Backup] ${emptyChecks.length} subsystem(s) exported ZERO rows — empty backup is not a usable ` +
234+
`recovery source: ${emptyChecks.map(c => c.subsystem).join(', ')}. Legitimate for a fresh ` +
235+
`environment; corruption-suspicious for a populated deployment.`
236+
);
237+
}
238+
226239
const completedAt = new Date().toISOString();
227240
const topology = buildTopologyDescriptor();
228241
const versionInfo = await buildVersionDescriptor(PROJECT_ROOT, logger);
229-
const meta = {
242+
const meta = {
230243
bundleVersion: 1,
231244
timestamp,
232245
completedAt,
@@ -246,15 +259,19 @@ export async function runBackup({
246259
* bundle. For subsystems whose `manageDatabaseBackup({action: 'export'})` SDK call returns a
247260
* numeric count (KB, MC memories+summaries, MC graph), this function counts non-empty lines
248261
* in the bundle's JSONL files and compares — mismatch indicates a partial/torn write that the
249-
* caller treats as a fail-the-bundle condition.
262+
* caller treats as a fail-the-bundle condition. Zero-zero parity (source and bundle both empty) is
263+
* reported as `empty`, not `pass`: a backup of an empty/gutted store is surfaced non-fatally because
264+
* it is not a usable recovery source (a fresh environment is legitimately empty; a populated
265+
* deployment reporting zero rows is the gutted-store signature). `runBackup` warns on `empty`
266+
* subsystems and persists the status into `bundle-meta.integrity` for a downstream canary/alert.
250267
*
251268
* For file-copy subsystems (concepts, trajectories, mailbox) the source side has no
252269
* authoritative count to compare against — `copyJsonlSource`'s reported `copied` field
253270
* already covers the file-presence check, so these are skipped with `status: 'skipped'`.
254271
*
255272
* @param {Object} layout The bundle's per-subsystem destination directory map.
256273
* @param {Object} subsystems The runBackup `subsystems` map of SDK return values.
257-
* @returns {Promise<Array<{subsystem: String, status: String, sourceCount: Number, bundleCount: Number, reason: String}>>} `status` is `pass` / `fail` / `skipped`; count + reason fields present per status.
274+
* @returns {Promise<Array<{subsystem: String, status: String, sourceCount: Number, bundleCount: Number, reason: String}>>} `status` is `pass` (positive row-count parity) / `empty` (source and bundle both zero — non-fatal, not a usable recovery source) / `fail` (row-count mismatch) / `skipped` (non-numeric source count); count + reason fields present per status.
258275
*/
259276
export async function verifyBundleIntegrity(layout, subsystems) {
260277
const verifiable = ['kb', 'mc', 'graph'];
@@ -276,16 +293,29 @@ export async function verifyBundleIntegrity(layout, subsystems) {
276293
continue;
277294
}
278295

279-
const files = (await fs.readdir(dir)).filter(f => f.endsWith('.jsonl'));
280-
let bundleCount = 0;
296+
const files = (await fs.readdir(dir)).filter(f => f.endsWith('.jsonl'));
297+
let bundleCount = 0;
281298

282299
for (const file of files) {
283300
const content = await fs.readFile(path.join(dir, file), 'utf8');
284301
bundleCount += content.split('\n').filter(line => line.trim()).length;
285302
}
286303

287304
if (bundleCount === sourceCount) {
288-
checks.push({subsystem, status: 'pass', sourceCount, bundleCount});
305+
// Empty-parity is NOT a healthy pass: a backup whose source AND bundle are both empty is
306+
// not a usable recovery source. Legitimate for a fresh environment, but for a normally-
307+
// populated deployment a zero-row export is the gutted-store signature (the corruption a
308+
// backup is supposed to survive) — surface it as 'empty' (visible, non-fatal) rather than
309+
// a silent 'pass' so a downstream canary/alert can act on bundle-meta.integrity.
310+
const status = sourceCount === 0 ? 'empty' : 'pass';
311+
const entry = {subsystem, status, sourceCount, bundleCount};
312+
313+
if (status === 'empty') {
314+
entry.reason = 'source and bundle both report zero rows — empty backup is not a usable ' +
315+
'recovery source (fresh-env legitimate; populated-deployment corruption-suspicious)';
316+
}
317+
318+
checks.push(entry);
289319
} else {
290320
checks.push({
291321
subsystem,
@@ -314,7 +344,7 @@ export async function verifyBundleIntegrity(layout, subsystems) {
314344
function buildTopologyDescriptor() {
315345
return {
316346
shared_topology: true,
317-
kbChromaCoords: {
347+
kbChromaCoords : {
318348
host: kbConfig.host ?? null,
319349
port: kbConfig.port ?? null,
320350
path: kbConfig.path ?? null
@@ -396,9 +426,9 @@ export async function cleanOldBackups(backupRoot, logger, retention = {}) {
396426
const tsMatch = entry.name.match(/^backup-(.+)$/);
397427
if (!tsMatch) continue;
398428

399-
const rawTs = tsMatch[1];
429+
const rawTs = tsMatch[1];
400430
const isoTime = rawTs.replace(/T(\d{2})-(\d{2})-(\d{2})/, 'T$1:$2:$3');
401-
const date = new Date(isoTime);
431+
const date = new Date(isoTime);
402432

403433
if (!isNaN(date.getTime())) {
404434
backups.push({
@@ -412,16 +442,16 @@ export async function cleanOldBackups(backupRoot, logger, retention = {}) {
412442

413443
backups.sort((a, b) => b.time - a.time);
414444

415-
const K = keepMinimum;
416-
const N_DAYS = maxDays;
417-
const now = Date.now();
445+
const K = keepMinimum;
446+
const N_DAYS = maxDays;
447+
const now = Date.now();
418448
const thresholdMs = N_DAYS * 24 * 60 * 60 * 1000;
419449

420450
let deletedCount = 0;
421451

422452
for (let i = K; i < backups.length; i++) {
423453
const backup = backups[i];
424-
const ageMs = now - backup.time;
454+
const ageMs = now - backup.time;
425455
if (ageMs > thresholdMs) {
426456
try {
427457
logger.log(`[Retention] Deleting old backup: ${backup.name} (age: ${Math.round(ageMs / 86400000)} days)`);

test/playwright/unit/ai/scripts/maintenance/backup.spec.mjs

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ test.describe('backup.mjs orchestrator — atomic bundle assembly (#10129 Phase
144144
});
145145

146146
test('reports missing concept/trajectory sources as non-fatal notes', async () => {
147-
const silentLogger = {log: () => {}, error: () => {}};
147+
const silentLogger = {log: () => {}, error: () => {}};
148148
const altBundleRoot = path.join(workRoot, 'bundle-no-optional-sources');
149149

150150
const result = await runBackup({
@@ -275,4 +275,58 @@ test.describe('backup.mjs orchestrator — atomic bundle assembly (#10129 Phase
275275
expect(mc.status).toBe('skipped');
276276
expect(mc.reason).toMatch(/no numeric source count/);
277277
});
278+
279+
test('verifyBundleIntegrity: empty (not a silent pass) when source and bundle are both zero (#14030)', async () => {
280+
const tempRoot = path.join(workRoot, 'integrity-empty');
281+
const kbDir = path.join(tempRoot, 'kb');
282+
283+
fs.mkdirSync(kbDir, {recursive: true});
284+
// The gutted-store signature: a populated deployment whose export came back empty. Both
285+
// sides agree at zero, so the old `bundleCount === sourceCount` branch reported 'pass' —
286+
// a false recovery source. It must surface as 'empty', never 'pass'.
287+
fs.writeFileSync(path.join(kbDir, 'kb-data.jsonl'), '');
288+
289+
const checks = await verifyBundleIntegrity(
290+
{kb: kbDir, mc: path.join(tempRoot, 'mc-missing'), graph: path.join(tempRoot, 'graph-missing')},
291+
{kb: 0}
292+
);
293+
294+
const kb = checks.find(c => c.subsystem === 'kb');
295+
expect(kb.status).toBe('empty');
296+
expect(kb.status).not.toBe('pass');
297+
expect(kb.sourceCount).toBe(0);
298+
expect(kb.bundleCount).toBe(0);
299+
expect(kb.reason).toMatch(/not a usable recovery source/);
300+
});
301+
302+
test('runBackup propagates an empty subsystem to bundle-meta.integrity "empty" + a non-fatal warning (#14048)', async () => {
303+
// Close-target proof: the helper status alone is not enough — runBackup must keep an empty
304+
// subsystem non-fatal, WARN, and persist `empty` into bundle-meta.integrity (the canary/alert
305+
// handoff). Force MC (memories + summaries) to export zero rows.
306+
const warnings = [];
307+
const captureLogger = {log: () => {}, error: () => {}, warn: msg => warnings.push(msg)};
308+
const savedMem = Memory_StorageRouter.getMemoryCollection;
309+
const savedSum = Memory_StorageRouter.getSummaryCollection;
310+
311+
try {
312+
Memory_StorageRouter.getMemoryCollection = async () => fakeCollection([], 'empty-mem');
313+
Memory_StorageRouter.getSummaryCollection = async () => fakeCollection([], 'empty-sum');
314+
315+
const result = await runBackup({
316+
bundleRoot: path.join(workRoot, 'bundle-empty-subsystem'),
317+
conceptsSourceDir,
318+
trajectoriesSourceFile,
319+
logger : captureLogger
320+
});
321+
322+
const mcIntegrity = result.meta.integrity.find(check => check.subsystem === 'mc');
323+
expect(mcIntegrity.status).toBe('empty'); // persisted into bundle-meta.integrity
324+
expect(mcIntegrity.sourceCount).toBe(0);
325+
expect(mcIntegrity.bundleCount).toBe(0);
326+
expect(warnings.some(w => /ZERO rows|empty backup/i.test(w))).toBe(true); // runBackup warned, non-fatally
327+
} finally {
328+
Memory_StorageRouter.getMemoryCollection = savedMem;
329+
Memory_StorageRouter.getSummaryCollection = savedSum;
330+
}
331+
});
278332
});

0 commit comments

Comments
 (0)