mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-30 03:12:32 +00:00
fix(sync): address adversarial review findings on the failure ledger (#1939)
- #1: a parse-failed file that is later deleted/renamed-away no longer leaves a permanent open ledger row. Removed paths (filtered.deleted, renamed-from, and the "gone from disk" forward-delete skip branch) are treated as resolved so the ledger self-heals instead of aging doctor to a stuck FAIL. - #3: decideSyncFailureSeverity escalates to FAIL on OPEN (blocking) failures only — auto_skipped rows already advanced the bookmark, so they stay WARN-visible regardless of count, matching the state-machine contract.
This commit is contained in:
+16
-1
@@ -1819,6 +1819,11 @@ async function performSyncInner(engine: BrainEngine, opts: SyncOpts): Promise<Sy
|
||||
// correct only when the gate compared HEAD == captured; under pinning a
|
||||
// forward delete must not block.
|
||||
await markCompleted(path);
|
||||
// issue #1939 adversarial finding #1: a file that previously failed to
|
||||
// parse (open ledger row) and is now gone from disk is resolved — clear
|
||||
// its row so it can't age doctor to a permanent FAIL. (This covers the
|
||||
// net-zero add-then-delete range where the path isn't in filtered.deleted.)
|
||||
succeededPaths.push(path);
|
||||
progress.tick(1, `skip:${path}`);
|
||||
return;
|
||||
}
|
||||
@@ -2030,10 +2035,20 @@ async function performSyncInner(engine: BrainEngine, opts: SyncOpts): Promise<Sy
|
||||
await clearOpCheckpoint(engine, ckpt.target);
|
||||
};
|
||||
|
||||
// issue #1939 adversarial finding #1: a file that failed to parse (open ledger
|
||||
// row) and is then deleted/renamed-away never re-enters failedFiles and never
|
||||
// imports, so its row would never clear and would age doctor to a permanent
|
||||
// FAIL. Treat removed paths as resolved so the ledger self-heals.
|
||||
const resolvedPaths = [
|
||||
...succeededPaths,
|
||||
...filtered.deleted,
|
||||
...filtered.renamed.map(r => r.from),
|
||||
];
|
||||
|
||||
const gate = await applySyncFailureGate({
|
||||
sourceId: opts.sourceId ?? DEFAULT_SOURCE_ID,
|
||||
failedFiles,
|
||||
succeededPaths,
|
||||
succeededPaths: resolvedPaths,
|
||||
commit: pin,
|
||||
skipFailed: opts.skipFailed === true,
|
||||
advance,
|
||||
|
||||
@@ -651,8 +651,11 @@ export function decideSyncFailureSeverity(args: {
|
||||
}
|
||||
const blockedTooLong =
|
||||
Number.isFinite(oldestOpenMs) && args.nowMs - oldestOpenMs > args.failHours * 3_600_000;
|
||||
const status: 'warn' | 'fail' =
|
||||
unresolved.length >= 10 || blockedTooLong ? 'fail' : 'warn';
|
||||
// FAIL keys off OPEN (blocking) failures only — many open, or one blocking the
|
||||
// bookmark past the fail cadence. `auto_skipped` rows already advanced the
|
||||
// bookmark (indexing is NOT wedged) so they stay WARN-visible regardless of
|
||||
// count, matching the state-machine contract. (#1939 adversarial finding #3.)
|
||||
const status: 'warn' | 'fail' = open >= 10 || blockedTooLong ? 'fail' : 'warn';
|
||||
return { status, unresolved: unresolved.length, open, auto_skipped: autoSkipped };
|
||||
}
|
||||
|
||||
|
||||
@@ -628,4 +628,30 @@ describeE2E('E2E: sync --skip-failed structured summary loop (v0.22.12, issue #5
|
||||
else process.env.GBRAIN_SYNC_AUTOSKIP_AFTER = prevThreshold;
|
||||
}
|
||||
});
|
||||
|
||||
// issue #1939 adversarial finding #1: a parse-failed file that is later DELETED
|
||||
// from the repo must not leave a permanent open ledger row (which would age
|
||||
// doctor to FAIL forever). The incremental gate treats removed paths as resolved.
|
||||
test('deleting a failed file clears its ledger row (self-heal, no stuck FAIL)', async () => {
|
||||
const { performSync } = await import('../../src/commands/sync.ts');
|
||||
const { loadSyncFailures } = await import('../../src/core/sync.ts');
|
||||
const engine = getEngine();
|
||||
|
||||
writeFileSync(join(repoPath, 'people/gonepoison.md'), [
|
||||
'---', 'type: person', 'title: Gone', 'slug: wrong-derived-slug', '---', '', 'Body.',
|
||||
].join('\n'));
|
||||
execSync('git add -A && git commit -m "add a file that fails to parse"', { cwd: repoPath, stdio: 'pipe' });
|
||||
|
||||
// Sync blocks; an open ledger row exists for the bad file.
|
||||
let result = await performSync(engine, { repoPath, noPull: true, noEmbed: true });
|
||||
expect(result.status).toBe('blocked_by_failures');
|
||||
expect(loadSyncFailures().some(f => f.path.includes('gonepoison'))).toBe(true);
|
||||
|
||||
// Delete the file and sync. The removed path is treated as resolved, so the
|
||||
// ledger row is cleared and the bookmark advances.
|
||||
execSync('git rm people/gonepoison.md && git commit -m "delete the bad file"', { cwd: repoPath, stdio: 'pipe' });
|
||||
result = await performSync(engine, { repoPath, noPull: true, noEmbed: true });
|
||||
expect(result.status).not.toBe('blocked_by_failures');
|
||||
expect(loadSyncFailures().some(f => f.path.includes('gonepoison'))).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -189,10 +189,17 @@ describe('#1 severity — auto_skipped stays visible (WARN)', () => {
|
||||
nowMs: now, failHours,
|
||||
}).status).toBe('fail');
|
||||
|
||||
// 10 recent open → fail (count)
|
||||
// 10 recent OPEN → fail (count of blocking failures)
|
||||
const ten = Array.from({ length: 10 }, (_, i) => ({ ...base, path: `a${i}.md`, state: 'open' as const, ts: '2026-06-06T23:00:00Z' }));
|
||||
expect(decideSyncFailureSeverity({ entries: ten, nowMs: now, failHours }).status).toBe('fail');
|
||||
|
||||
// 10 recent AUTO_SKIPPED → still warn (#3): the valve already advanced the
|
||||
// bookmark, so indexing is not wedged. Visible, not gating.
|
||||
const tenSkipped = Array.from({ length: 10 }, (_, i) => ({ ...base, path: `s${i}.md`, state: 'auto_skipped' as const, ts: '2026-06-06T23:00:00Z' }));
|
||||
const skSev = decideSyncFailureSeverity({ entries: tenSkipped, nowMs: now, failHours });
|
||||
expect(skSev.status).toBe('warn');
|
||||
expect(skSev.auto_skipped).toBe(10);
|
||||
|
||||
// auto_skipped only → warn (still visible), counted
|
||||
const sev = decideSyncFailureSeverity({
|
||||
entries: [{ ...base, state: 'auto_skipped', ts: '2026-06-01T00:00:00Z' }],
|
||||
|
||||
Reference in New Issue
Block a user