mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 22:15:33 +00:00
v0.42.32.0 fix(sync): coerce non-string frontmatter titles + bounded auto-skip failure ledger (#1939) (#1956)
* fix(import): coerce non-string frontmatter title/slug/type (#1939) YAML `title: 2024-06-01` parses to a Date and `title: 1458` to a number; the old `(frontmatter.X as string)` cast was a compile-time lie, so downstream `.toLowerCase()` threw and (via the importer failure gate) could wedge sync indefinitely. parseMarkdown now coerces via coerceFrontmatterString (Date -> UTC ISO date, deterministic), and the pure assessContentSanity self-protects against a non-string title. * feat(sync): bounded auto-skip failure ledger; poison file can't wedge indexing (#1939) New src/core/sync-failure-ledger.ts owns the failure store + a crash-safe, multi-source, concurrent bounded auto-skip valve. A file that fails N consecutive syncs (GBRAIN_SYNC_AUTOSKIP_AFTER, default 3) auto-skips so it can't freeze all indexing forever, while fresh failures still fail-closed and a `<head>` history-rewrite sentinel hard-blocks even with --skip-failed. - (source_id, path) keying — failures never merge across sources - success clears a path so attempts are truly consecutive - advance-before-ack ordering (a crash can't mark a file skipped while wedged) - shared applySyncFailureGate used by BOTH the incremental and full-sync gates - legacy-row normalization + duplicate collapse on load - cross-process lock + atomic temp-rename, age-based stale-lock break sync.ts re-exports the ledger for existing callers; import.ts records source-scoped and defers the bookmark to the gate under managedBookmark. * fix(doctor): sync_failures severity via one shared decision on both surfaces (#1939) Local buildChecks and remote doctorReportRemote now both route through decideSyncFailureSeverity, so a stuck bookmark escalates WARN -> FAIL consistently (oldest-open age > fail cadence, or large unresolved count), auto-skipped pages stay visible (WARN, not hidden), and the acknowledged/acknowledged_at field-split that caused drift is gone. The remote surface stays subprocess-free (file read + Date.parse only). * chore(test): add trailing newline to e5-lease-cap-ab baseline fixture * 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. * chore: bump version and changelog (v0.42.30.0) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: document sync-failure ledger + auto-skip valve for v0.42.30.0 KEY_FILES.md: new src/core/sync-failure-ledger.ts entry (bounded auto-skip state machine, decideGateAction/decideSyncFailureSeverity/applySyncFailureGate, GBRAIN_SYNC_AUTOSKIP_AFTER); update sync.ts (failure store moved to ledger, re-exported), doctor.ts (sync_failures severity via shared rule on both surfaces), markdown.ts (coerceFrontmatterString), import.ts (managedBookmark). live-sync.md: poison-file auto-skip tricky-spot. Regenerated llms-full.txt. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: re-bump to v0.42.31.0 (queue collision on 0.42.30.0) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: re-bump to v0.42.32.0 (queue collision) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Garry Tan
co-authored by
Claude Opus 4.8
Claude Opus 4.8
parent
f401d7407e
commit
5a06af5a57
@@ -640,3 +640,28 @@ describe('assessContentSanity — confidence split (Q1=A)', () => {
|
||||
expect(r.shouldFlag).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// issue #1939 — assessContentSanity is a pure exported fn; lint.ts and
|
||||
// import-file both pass `parsed.title`, which a malformed YAML date/number title
|
||||
// could make non-string. It must coerce defensively and never throw.
|
||||
describe('issue #1939 — non-string title defensive coercion', () => {
|
||||
const base = { compiled_truth: 'some prose body here', timeline: '' };
|
||||
|
||||
test('Date title does not throw', () => {
|
||||
expect(() =>
|
||||
assessContentSanity({ ...base, title: new Date('2024-06-01') as unknown as string }),
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
test('number title does not throw', () => {
|
||||
expect(() =>
|
||||
assessContentSanity({ ...base, title: 1458 as unknown as string }),
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
test('null/undefined title does not throw and yields a normal result', () => {
|
||||
const r = assessContentSanity({ ...base, title: undefined as unknown as string });
|
||||
expect(r).toBeDefined();
|
||||
expect(typeof r.shouldQuarantine).toBe('boolean');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -553,4 +553,105 @@ describeE2E('E2E: sync --skip-failed structured summary loop (v0.22.12, issue #5
|
||||
const finalSummary = summarizeFailuresByCode(failures);
|
||||
expect(finalSummary).toEqual([{ code: 'SLUG_MISMATCH', count: 2 }]);
|
||||
});
|
||||
|
||||
// issue #1939 CRITICAL REGRESSION: a page whose YAML title parses to a Date
|
||||
// (or number) must import cleanly — pre-fix it threw in assessContentSanity
|
||||
// and wedged the bookmark. This mirrors the apple-notes repro
|
||||
// (sources/apple-notes/.../2024-06-01 8189165238.md).
|
||||
test('date/number-titled page imports cleanly; bookmark advances; get returns it', async () => {
|
||||
const { performSync } = await import('../../src/commands/sync.ts');
|
||||
const { loadSyncFailures } = await import('../../src/core/sync.ts');
|
||||
const engine = getEngine();
|
||||
|
||||
const beforeCommit = await engine.getConfig('sync.last_commit');
|
||||
// Bare-date title (→ Date) and bare-number title (→ number).
|
||||
writeFileSync(join(repoPath, 'people/datey.md'), [
|
||||
'---', 'type: note', 'title: 2024-06-01', '---', '', 'Apple note body.',
|
||||
].join('\n'));
|
||||
writeFileSync(join(repoPath, 'people/numbery.md'), [
|
||||
'---', 'type: note', 'title: 1458', '---', '', 'Another note.',
|
||||
].join('\n'));
|
||||
execSync('git add -A && git commit -m "add date/number titled notes"', { cwd: repoPath, stdio: 'pipe' });
|
||||
|
||||
const result = await performSync(engine, { repoPath, noPull: true, noEmbed: true });
|
||||
expect(result.status).not.toBe('blocked_by_failures');
|
||||
|
||||
// Neither file landed as a failure.
|
||||
const fails = loadSyncFailures().filter(f => f.path.includes('datey') || f.path.includes('numbery'));
|
||||
expect(fails.length).toBe(0);
|
||||
|
||||
// Bookmark advanced past the broken-but-now-fixed commit.
|
||||
const afterCommit = await engine.getConfig('sync.last_commit');
|
||||
expect(afterCommit).not.toBe(beforeCommit);
|
||||
|
||||
// The pages are retrievable, with deterministic coerced titles.
|
||||
const datey = await engine.getPage('people/datey');
|
||||
expect(datey).not.toBeNull();
|
||||
expect(datey!.title).toBe('2024-06-01');
|
||||
const numbery = await engine.getPage('people/numbery');
|
||||
expect(numbery).not.toBeNull();
|
||||
expect(numbery!.title).toBe('1458');
|
||||
});
|
||||
|
||||
// issue #1939 valve: a genuinely un-importable file blocks for (threshold-1)
|
||||
// syncs, then auto-skips on the Nth so it can't wedge indexing forever.
|
||||
test('bounded auto-skip: poison file blocks then auto-skips, advancing the bookmark', async () => {
|
||||
const { performSync } = await import('../../src/commands/sync.ts');
|
||||
const { loadSyncFailures } = await import('../../src/core/sync.ts');
|
||||
const engine = getEngine();
|
||||
const prevThreshold = process.env.GBRAIN_SYNC_AUTOSKIP_AFTER;
|
||||
process.env.GBRAIN_SYNC_AUTOSKIP_AFTER = '2';
|
||||
try {
|
||||
const beforeCommit = await engine.getConfig('sync.last_commit');
|
||||
// slug-mismatch = a reliable per-file import failure.
|
||||
writeFileSync(join(repoPath, 'people/poison.md'), [
|
||||
'---', 'type: person', 'title: Poison', 'slug: not-the-path-slug', '---', '', 'Body.',
|
||||
].join('\n'));
|
||||
execSync('git add -A && git commit -m "add poison file"', { cwd: repoPath, stdio: 'pipe' });
|
||||
|
||||
// Attempt 1: blocks (attempts=1 < 2).
|
||||
let result = await performSync(engine, { repoPath, noPull: true, noEmbed: true });
|
||||
expect(result.status).toBe('blocked_by_failures');
|
||||
expect(await engine.getConfig('sync.last_commit')).toBe(beforeCommit);
|
||||
let poison = loadSyncFailures().find(f => f.path.includes('poison'))!;
|
||||
expect(poison.state).toBe('open');
|
||||
expect(poison.attempts).toBe(1);
|
||||
|
||||
// Attempt 2: attempts hits threshold, fresh==0 → auto-skip + advance.
|
||||
result = await performSync(engine, { repoPath, noPull: true, noEmbed: true });
|
||||
expect(result.status).toBe('synced');
|
||||
expect(await engine.getConfig('sync.last_commit')).not.toBe(beforeCommit);
|
||||
poison = loadSyncFailures().find(f => f.path.includes('poison'))!;
|
||||
expect(poison.state).toBe('auto_skipped');
|
||||
} finally {
|
||||
if (prevThreshold === undefined) delete process.env.GBRAIN_SYNC_AUTOSKIP_AFTER;
|
||||
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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -40,4 +40,4 @@
|
||||
"pr_gate_pass": false,
|
||||
"note": "controller does NOT meet PR gate; defaults stay OFF"
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -302,3 +302,44 @@ Some content.`;
|
||||
expect(parseMarkdown('', 'projects/blog/writing/essay.md').type).toBe('writing');
|
||||
});
|
||||
});
|
||||
|
||||
// issue #1939 — js-yaml parses `title: 2024-06-01` as a Date and `title: 1458`
|
||||
// as a number. The old `(frontmatter.title as string)` cast was a compile-time
|
||||
// lie; at runtime downstream `.toLowerCase()` threw and wedged sync. Coercion
|
||||
// must be non-throwing AND deterministic (UTC ISO for dates, no timezone drift).
|
||||
describe('issue #1939 — non-string frontmatter coercion', () => {
|
||||
test('date title coerces to its UTC ISO date string', () => {
|
||||
const parsed = parseMarkdown('---\ntitle: 2024-06-01\n---\nbody\n', 'apple-notes/x.md');
|
||||
expect(parsed.title).toBe('2024-06-01');
|
||||
expect(typeof parsed.title).toBe('string');
|
||||
});
|
||||
|
||||
test('number title coerces to its string form', () => {
|
||||
const parsed = parseMarkdown('---\ntitle: 1458\n---\nbody\n', 'apple-notes/x.md');
|
||||
expect(parsed.title).toBe('1458');
|
||||
});
|
||||
|
||||
test('date title is timezone-independent (UTC) — repro file shape', () => {
|
||||
// sources/apple-notes/YC/Talks YC/2023-04-25 1458.md style page.
|
||||
const parsed = parseMarkdown('---\ntitle: 2023-04-25\n---\nnotes\n', 'apple-notes/2023-04-25 1458.md');
|
||||
expect(parsed.title).toBe('2023-04-25'); // never "Mon Apr 24 2023 ...GMT-0700"
|
||||
});
|
||||
|
||||
test('date/number slug + type coerce without throwing', () => {
|
||||
const parsed = parseMarkdown('---\nslug: 2024-06-01\ntype: 2024\n---\nbody\n', 'x.md');
|
||||
expect(typeof parsed.slug).toBe('string');
|
||||
expect(parsed.slug).toBe('2024-06-01');
|
||||
expect(typeof parsed.type).toBe('string');
|
||||
});
|
||||
|
||||
test('missing/empty title falls back to inferred title (no throw)', () => {
|
||||
const parsed = parseMarkdown('---\ntype: note\n---\nbody\n', 'people/alice-example.md');
|
||||
expect(typeof parsed.title).toBe('string');
|
||||
expect(parsed.title.length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
test('string title still passes through unchanged', () => {
|
||||
const parsed = parseMarkdown('---\ntitle: A Normal Title\n---\nbody\n', 'x.md');
|
||||
expect(parsed.title).toBe('A Normal Title');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,377 @@
|
||||
/**
|
||||
* issue #1939 — sync failure ledger + bounded auto-skip valve.
|
||||
*
|
||||
* Covers the correctness gates the /codex outside-voice review identified:
|
||||
* #1 auto-skipped entries stay UNRESOLVED (doctor WARN), not hidden
|
||||
* #2 (source_id, path) keying — failures never merge across sources
|
||||
* #3 `<head>` sentinel never auto-skips; always hard-blocks
|
||||
* #4 success clears a path so `attempts` is truly consecutive
|
||||
* #5 advance-before-ack atomicity (a throwing advance marks nothing)
|
||||
* #7 legacy rows normalize + duplicates collapse deterministically
|
||||
* #8 cross-process lock + atomic write (stale-lock break, no partial file)
|
||||
*/
|
||||
|
||||
import { describe, test, expect, beforeEach, afterEach } from 'bun:test';
|
||||
import { mkdtempSync, rmSync, readFileSync, writeFileSync, mkdirSync, existsSync, utimesSync, openSync, closeSync } from 'fs';
|
||||
import { join, dirname } from 'path';
|
||||
import { tmpdir } from 'os';
|
||||
|
||||
let tmpHome: string;
|
||||
const originalHome = process.env.HOME;
|
||||
const originalThreshold = process.env.GBRAIN_SYNC_AUTOSKIP_AFTER;
|
||||
|
||||
beforeEach(async () => {
|
||||
tmpHome = mkdtempSync(join(tmpdir(), 'gbrain-ledger-'));
|
||||
process.env.HOME = tmpHome;
|
||||
delete process.env.GBRAIN_SYNC_AUTOSKIP_AFTER;
|
||||
const { syncFailuresPath } = await import('../src/core/sync-failure-ledger.ts');
|
||||
try { rmSync(syncFailuresPath(), { force: true }); } catch { /* none */ }
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (originalHome) process.env.HOME = originalHome; else delete process.env.HOME;
|
||||
if (originalThreshold === undefined) delete process.env.GBRAIN_SYNC_AUTOSKIP_AFTER;
|
||||
else process.env.GBRAIN_SYNC_AUTOSKIP_AFTER = originalThreshold;
|
||||
try { rmSync(tmpHome, { recursive: true, force: true }); } catch { /* ignore */ }
|
||||
});
|
||||
|
||||
async function L() {
|
||||
return import('../src/core/sync-failure-ledger.ts');
|
||||
}
|
||||
|
||||
describe('#2 multi-source keying', () => {
|
||||
test('same relative path in two sources keeps independent counters', async () => {
|
||||
const { recordFailures, loadSyncFailures } = await L();
|
||||
recordFailures('alpha', [{ path: 'people/x.md', error: 'YAML parse failed' }], 'c1');
|
||||
recordFailures('alpha', [{ path: 'people/x.md', error: 'YAML parse failed' }], 'c2');
|
||||
recordFailures('beta', [{ path: 'people/x.md', error: 'YAML parse failed' }], 'c1');
|
||||
|
||||
const rows = loadSyncFailures();
|
||||
expect(rows.length).toBe(2);
|
||||
const alpha = rows.find(r => r.source_id === 'alpha')!;
|
||||
const beta = rows.find(r => r.source_id === 'beta')!;
|
||||
expect(alpha.attempts).toBe(2);
|
||||
expect(beta.attempts).toBe(1);
|
||||
});
|
||||
|
||||
test('acknowledgeFailures(sourceId) only acks that source', async () => {
|
||||
const { recordFailures, acknowledgeFailures, loadSyncFailures } = await L();
|
||||
recordFailures('alpha', [{ path: 'a.md', error: 'e' }], 'c1');
|
||||
recordFailures('beta', [{ path: 'b.md', error: 'e' }], 'c1');
|
||||
|
||||
const res = acknowledgeFailures('alpha');
|
||||
expect(res.count).toBe(1);
|
||||
const rows = loadSyncFailures();
|
||||
expect(rows.find(r => r.source_id === 'alpha')!.state).toBe('acknowledged');
|
||||
expect(rows.find(r => r.source_id === 'beta')!.state).toBe('open');
|
||||
});
|
||||
});
|
||||
|
||||
describe('#4 success clears → consecutive attempts', () => {
|
||||
test('clearFailures removes a path; a later failure restarts at 1', async () => {
|
||||
const { recordFailures, clearFailures, loadSyncFailures } = await L();
|
||||
recordFailures('s', [{ path: 'a.md', error: 'e' }], 'c1');
|
||||
recordFailures('s', [{ path: 'a.md', error: 'e' }], 'c2');
|
||||
expect(loadSyncFailures()[0].attempts).toBe(2);
|
||||
|
||||
clearFailures('s', ['a.md']);
|
||||
expect(loadSyncFailures().length).toBe(0);
|
||||
|
||||
recordFailures('s', [{ path: 'a.md', error: 'e' }], 'c3');
|
||||
expect(loadSyncFailures()[0].attempts).toBe(1);
|
||||
});
|
||||
|
||||
test('clearFailures is source-scoped', async () => {
|
||||
const { recordFailures, clearFailures, loadSyncFailures } = await L();
|
||||
recordFailures('s1', [{ path: 'a.md', error: 'e' }], 'c1');
|
||||
recordFailures('s2', [{ path: 'a.md', error: 'e' }], 'c1');
|
||||
clearFailures('s1', ['a.md']);
|
||||
const rows = loadSyncFailures();
|
||||
expect(rows.length).toBe(1);
|
||||
expect(rows[0].source_id).toBe('s2');
|
||||
});
|
||||
});
|
||||
|
||||
describe('#3 sentinel never auto-skips', () => {
|
||||
test('decideGateAction hard-blocks on a sentinel even when chronic + skipFailed', async () => {
|
||||
const { decideGateAction } = await L();
|
||||
const d = decideGateAction({
|
||||
fileFailures: [],
|
||||
sentinels: [{ path: '<head>' }],
|
||||
attemptsByPath: new Map(),
|
||||
threshold: 3,
|
||||
skipFailed: true,
|
||||
});
|
||||
expect(d.action).toBe('hard_block');
|
||||
expect(d.autoSkipPaths).toEqual([]);
|
||||
});
|
||||
|
||||
test('autoSkipFailures refuses to skip a sentinel path', async () => {
|
||||
const { recordFailures, autoSkipFailures, loadSyncFailures } = await L();
|
||||
recordFailures('s', [{ path: '<head>', error: 'history rewrite' }], 'c1');
|
||||
const res = autoSkipFailures('s', ['<head>']);
|
||||
expect(res.count).toBe(0);
|
||||
expect(loadSyncFailures()[0].state).toBe('open');
|
||||
});
|
||||
|
||||
test('isSkippablePath', async () => {
|
||||
const { isSkippablePath } = await L();
|
||||
expect(isSkippablePath('people/x.md')).toBe(true);
|
||||
expect(isSkippablePath('<head>')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('#7 legacy normalization + dup collapse', () => {
|
||||
test('backfills source_id/state/attempts/first_seen on legacy rows', async () => {
|
||||
const { loadSyncFailures, syncFailuresPath } = await L();
|
||||
mkdirSync(dirname(syncFailuresPath()), { recursive: true });
|
||||
writeFileSync(
|
||||
syncFailuresPath(),
|
||||
JSON.stringify({ path: 'a.md', error: 'YAML parse failed', commit: 'old', ts: '2025-01-01T00:00:00Z' }) + '\n' +
|
||||
JSON.stringify({ path: 'b.md', error: 'e', commit: 'old', ts: '2025-01-02T00:00:00Z', acknowledged_at: '2025-02-01T00:00:00Z' }) + '\n',
|
||||
);
|
||||
const rows = loadSyncFailures();
|
||||
const a = rows.find(r => r.path === 'a.md')!;
|
||||
const b = rows.find(r => r.path === 'b.md')!;
|
||||
expect(a.source_id).toBe('default');
|
||||
expect(a.state).toBe('open');
|
||||
expect(a.attempts).toBe(1);
|
||||
expect(a.first_seen).toBe('2025-01-01T00:00:00Z');
|
||||
expect(b.state).toBe('acknowledged');
|
||||
});
|
||||
|
||||
test('collapses duplicate open rows for one (source,path) into one with attempts = distinct commits', async () => {
|
||||
const { loadSyncFailures, syncFailuresPath } = await L();
|
||||
mkdirSync(dirname(syncFailuresPath()), { recursive: true });
|
||||
// Three legacy rows, same path, two distinct commits.
|
||||
writeFileSync(
|
||||
syncFailuresPath(),
|
||||
[
|
||||
{ path: 'a.md', error: 'e', commit: 'c1', ts: '2025-01-01T00:00:00Z' },
|
||||
{ path: 'a.md', error: 'e', commit: 'c2', ts: '2025-01-02T00:00:00Z' },
|
||||
{ path: 'a.md', error: 'e2', commit: 'c2', ts: '2025-01-03T00:00:00Z' },
|
||||
].map(r => JSON.stringify(r)).join('\n') + '\n',
|
||||
);
|
||||
const rows = loadSyncFailures();
|
||||
expect(rows.length).toBe(1);
|
||||
expect(rows[0].attempts).toBe(2); // distinct commits c1, c2
|
||||
expect(rows[0].commit).toBe('c2'); // latest by ts
|
||||
expect(rows[0].first_seen).toBe('2025-01-01T00:00:00Z');
|
||||
});
|
||||
|
||||
test('skips malformed lines', async () => {
|
||||
const { loadSyncFailures, recordFailures, syncFailuresPath } = await L();
|
||||
recordFailures('s', [{ path: 'a.md', error: 'e' }], 'c1');
|
||||
writeFileSync(syncFailuresPath(), readFileSync(syncFailuresPath(), 'utf-8') + 'NOT-JSON\n');
|
||||
expect(loadSyncFailures().length).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('#1 severity — auto_skipped stays visible (WARN)', () => {
|
||||
test('decideSyncFailureSeverity branches', async () => {
|
||||
const { decideSyncFailureSeverity } = await L();
|
||||
const base = { source_id: 's', path: 'a.md', error: 'e', code: 'X', commit: 'c', first_seen: '', ts: '', attempts: 1 };
|
||||
const now = Date.parse('2026-06-07T00:00:00Z');
|
||||
const failHours = 72;
|
||||
|
||||
// empty → ok
|
||||
expect(decideSyncFailureSeverity({ entries: [], nowMs: now, failHours }).status).toBe('ok');
|
||||
|
||||
// one recent open → warn
|
||||
expect(decideSyncFailureSeverity({
|
||||
entries: [{ ...base, state: 'open', ts: '2026-06-06T18:00:00Z' }],
|
||||
nowMs: now, failHours,
|
||||
}).status).toBe('warn');
|
||||
|
||||
// one OLD open (>72h) → fail
|
||||
expect(decideSyncFailureSeverity({
|
||||
entries: [{ ...base, state: 'open', ts: '2026-06-01T00:00:00Z' }],
|
||||
nowMs: now, failHours,
|
||||
}).status).toBe('fail');
|
||||
|
||||
// 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' }],
|
||||
nowMs: now, failHours,
|
||||
});
|
||||
expect(sev.status).toBe('warn');
|
||||
expect(sev.auto_skipped).toBe(1);
|
||||
expect(sev.unresolved).toBe(1);
|
||||
|
||||
// acknowledged only → ok
|
||||
expect(decideSyncFailureSeverity({
|
||||
entries: [{ ...base, state: 'acknowledged', ts: '2026-06-01T00:00:00Z' }],
|
||||
nowMs: now, failHours,
|
||||
}).status).toBe('ok');
|
||||
});
|
||||
|
||||
test('malformed ts never crashes (treated as not-old)', async () => {
|
||||
const { decideSyncFailureSeverity } = await L();
|
||||
const sev = decideSyncFailureSeverity({
|
||||
entries: [{ source_id: 's', path: 'a.md', error: 'e', code: 'X', commit: 'c', first_seen: '', ts: 'not-a-date', attempts: 1, state: 'open' }],
|
||||
nowMs: Date.now(), failHours: 72,
|
||||
});
|
||||
expect(sev.status).toBe('warn');
|
||||
});
|
||||
|
||||
test('auto_skipped row keeps acknowledged_at null so legacy !acknowledged_at readers still count it', async () => {
|
||||
const { recordFailures, autoSkipFailures, loadSyncFailures } = await L();
|
||||
recordFailures('s', [{ path: 'a.md', error: 'e' }], 'c1');
|
||||
autoSkipFailures('s', ['a.md']);
|
||||
const row = loadSyncFailures()[0];
|
||||
expect(row.state).toBe('auto_skipped');
|
||||
expect(row.acknowledged).toBe(false);
|
||||
expect(row.acknowledged_at).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('decideGateAction — full branch table', () => {
|
||||
test('all branches', async () => {
|
||||
const { decideGateAction } = await L();
|
||||
const mk = (over: Partial<Parameters<typeof decideGateAction>[0]>) => decideGateAction({
|
||||
fileFailures: [], sentinels: [], attemptsByPath: new Map(), threshold: 3, skipFailed: false, ...over,
|
||||
});
|
||||
|
||||
expect(mk({}).action).toBe('advance'); // no failures
|
||||
expect(mk({ fileFailures: [{ path: 'a' }], skipFailed: true }).action).toBe('advance');
|
||||
expect(mk({ fileFailures: [{ path: 'a' }], threshold: 0 }).action).toBe('block'); // valve off
|
||||
expect(mk({ fileFailures: [{ path: 'a' }], attemptsByPath: new Map([['a', 1]]) }).action).toBe('block'); // fresh
|
||||
|
||||
const chronic = mk({ fileFailures: [{ path: 'a' }], attemptsByPath: new Map([['a', 3]]) });
|
||||
expect(chronic.action).toBe('advance_then_autoskip');
|
||||
expect(chronic.autoSkipPaths).toEqual(['a']);
|
||||
|
||||
// mixed fresh + chronic → block (don't silently drop the fresh one)
|
||||
expect(mk({
|
||||
fileFailures: [{ path: 'a' }, { path: 'b' }],
|
||||
attemptsByPath: new Map([['a', 3], ['b', 1]]),
|
||||
}).action).toBe('block');
|
||||
});
|
||||
});
|
||||
|
||||
describe('#5 + #6 applySyncFailureGate orchestration', () => {
|
||||
test('advance_then_autoskip after threshold; advance runs before auto-skip mark', async () => {
|
||||
process.env.GBRAIN_SYNC_AUTOSKIP_AFTER = '3';
|
||||
const { applySyncFailureGate, loadSyncFailures } = await L();
|
||||
let advanceCalls = 0;
|
||||
const run = () => applySyncFailureGate({
|
||||
sourceId: 's', failedFiles: [{ path: 'poison.md', error: 'YAML parse failed' }],
|
||||
succeededPaths: [], commit: 'c', skipFailed: false,
|
||||
advance: async () => { advanceCalls++; },
|
||||
});
|
||||
|
||||
let r = await run();
|
||||
expect(r.advanced).toBe(false); // attempts 1 → block
|
||||
r = await run();
|
||||
expect(r.advanced).toBe(false); // attempts 2 → block
|
||||
r = await run();
|
||||
expect(r.advanced).toBe(true); // attempts 3 → advance + auto-skip
|
||||
expect(r.autoSkipped).toEqual(['poison.md']);
|
||||
expect(advanceCalls).toBe(1);
|
||||
expect(loadSyncFailures()[0].state).toBe('auto_skipped');
|
||||
});
|
||||
|
||||
test('a throwing advance() marks nothing auto_skipped (atomicity)', async () => {
|
||||
process.env.GBRAIN_SYNC_AUTOSKIP_AFTER = '1';
|
||||
const { applySyncFailureGate, loadSyncFailures } = await L();
|
||||
await expect(applySyncFailureGate({
|
||||
sourceId: 's', failedFiles: [{ path: 'poison.md', error: 'e' }],
|
||||
succeededPaths: [], commit: 'c', skipFailed: false,
|
||||
advance: async () => { throw new Error('db write failed'); },
|
||||
})).rejects.toThrow('db write failed');
|
||||
// Recorded as open, NOT auto_skipped — next run can retry.
|
||||
const row = loadSyncFailures()[0];
|
||||
expect(row.state).toBe('open');
|
||||
});
|
||||
|
||||
test('sentinel hard-blocks even with skipFailed; no advance', async () => {
|
||||
const { applySyncFailureGate } = await L();
|
||||
let advanced = false;
|
||||
const r = await applySyncFailureGate({
|
||||
sourceId: 's', failedFiles: [{ path: '<head>', error: 'history rewrite' }],
|
||||
succeededPaths: [], commit: 'c', skipFailed: true,
|
||||
advance: async () => { advanced = true; },
|
||||
});
|
||||
expect(r.advanced).toBe(false);
|
||||
expect(r.sentinelBlocked).toBe(true);
|
||||
expect(advanced).toBe(false);
|
||||
});
|
||||
|
||||
test('succeeded paths clear prior failures even with no new failures', async () => {
|
||||
const { recordFailures, applySyncFailureGate, loadSyncFailures } = await L();
|
||||
recordFailures('s', [{ path: 'a.md', error: 'e' }], 'c1');
|
||||
expect(loadSyncFailures().length).toBe(1);
|
||||
const r = await applySyncFailureGate({
|
||||
sourceId: 's', failedFiles: [], succeededPaths: ['a.md'], commit: 'c2',
|
||||
skipFailed: false, advance: async () => {},
|
||||
});
|
||||
expect(r.advanced).toBe(true);
|
||||
expect(loadSyncFailures().length).toBe(0);
|
||||
});
|
||||
|
||||
test('skipFailed acknowledges and advances', async () => {
|
||||
const { applySyncFailureGate, loadSyncFailures } = await L();
|
||||
const r = await applySyncFailureGate({
|
||||
sourceId: 's', failedFiles: [{ path: 'a.md', error: 'e' }],
|
||||
succeededPaths: [], commit: 'c', skipFailed: true, advance: async () => {},
|
||||
});
|
||||
expect(r.advanced).toBe(true);
|
||||
expect(r.acknowledged).toBe(1);
|
||||
expect(loadSyncFailures()[0].state).toBe('acknowledged');
|
||||
});
|
||||
});
|
||||
|
||||
describe('#8 concurrency: lock + atomic write', () => {
|
||||
test('atomic rewrite leaves valid JSON lines (no partial)', async () => {
|
||||
const { recordFailures, syncFailuresPath } = await L();
|
||||
recordFailures('s', [{ path: 'a.md', error: 'e' }, { path: 'b.md', error: 'e' }], 'c1');
|
||||
const raw = readFileSync(syncFailuresPath(), 'utf-8');
|
||||
for (const line of raw.split('\n').filter(Boolean)) {
|
||||
expect(() => JSON.parse(line)).not.toThrow();
|
||||
}
|
||||
expect(existsSync(syncFailuresPath() + '.lock')).toBe(false); // lock released
|
||||
});
|
||||
|
||||
test('a stale lock (old mtime) is broken so a mutation still proceeds', async () => {
|
||||
const { recordFailures, syncFailuresPath, withLedgerLock } = await L();
|
||||
// Pre-create the data dir + a stale lock file.
|
||||
mkdirSync(dirname(syncFailuresPath()), { recursive: true });
|
||||
const lockPath = syncFailuresPath() + '.lock';
|
||||
closeSync(openSync(lockPath, 'w'));
|
||||
const old = new Date(Date.now() - 120_000); // 2 min ago > 30s stale window
|
||||
utimesSync(lockPath, old, old);
|
||||
|
||||
// Mutation should break the stale lock and succeed.
|
||||
recordFailures('s', [{ path: 'a.md', error: 'e' }], 'c1');
|
||||
const { loadSyncFailures } = await L();
|
||||
expect(loadSyncFailures().length).toBe(1);
|
||||
|
||||
// withLedgerLock returns the callback's value.
|
||||
expect(withLedgerLock(() => 42)).toBe(42);
|
||||
});
|
||||
});
|
||||
|
||||
describe('resolveAutoSkipThreshold', () => {
|
||||
test('default 3, env override, 0 disables, invalid → default', async () => {
|
||||
const { resolveAutoSkipThreshold } = await L();
|
||||
delete process.env.GBRAIN_SYNC_AUTOSKIP_AFTER;
|
||||
expect(resolveAutoSkipThreshold()).toBe(3);
|
||||
process.env.GBRAIN_SYNC_AUTOSKIP_AFTER = '5';
|
||||
expect(resolveAutoSkipThreshold()).toBe(5);
|
||||
process.env.GBRAIN_SYNC_AUTOSKIP_AFTER = '0';
|
||||
expect(resolveAutoSkipThreshold()).toBe(0);
|
||||
process.env.GBRAIN_SYNC_AUTOSKIP_AFTER = 'nonsense';
|
||||
expect(resolveAutoSkipThreshold()).toBe(3);
|
||||
});
|
||||
});
|
||||
+39
-14
@@ -40,7 +40,11 @@ afterEach(() => {
|
||||
});
|
||||
|
||||
describe('Bug 9 — sync-failures JSONL helpers', () => {
|
||||
test('recordSyncFailures appends one line per failure with dedup', async () => {
|
||||
// issue #1939: recordSyncFailures now upserts by (source_id, path) and
|
||||
// increments `attempts` (consecutive failed runs) instead of appending a row
|
||||
// per (path, commit, error). One row per failing path; the attempt counter
|
||||
// drives the bounded auto-skip valve.
|
||||
test('recordSyncFailures upserts per path, incrementing attempts', async () => {
|
||||
const { recordSyncFailures, loadSyncFailures, syncFailuresPath } = await import('../src/core/sync.ts');
|
||||
|
||||
recordSyncFailures([
|
||||
@@ -51,21 +55,28 @@ describe('Bug 9 — sync-failures JSONL helpers', () => {
|
||||
expect(existsSync(syncFailuresPath())).toBe(true);
|
||||
const entries = loadSyncFailures();
|
||||
expect(entries.length).toBe(2);
|
||||
expect(entries[0].path).toBe('people/alice.md');
|
||||
expect(entries[0].commit).toBe('abc123def456');
|
||||
expect(entries[0].acknowledged).toBeUndefined();
|
||||
const alice = entries.find(e => e.path === 'people/alice.md')!;
|
||||
expect(alice.commit).toBe('abc123def456');
|
||||
expect(alice.state).toBe('open');
|
||||
expect(alice.attempts).toBe(1);
|
||||
expect(alice.acknowledged).toBe(false);
|
||||
|
||||
// Same failure on same commit should NOT re-append.
|
||||
// Same path again (same commit) → still ONE row, attempts climbs.
|
||||
recordSyncFailures([
|
||||
{ path: 'people/alice.md', error: 'YAML: unexpected colon in title' },
|
||||
], 'abc123def456');
|
||||
expect(loadSyncFailures().length).toBe(2);
|
||||
expect(loadSyncFailures().find(e => e.path === 'people/alice.md')!.attempts).toBe(2);
|
||||
|
||||
// Different commit → new entry.
|
||||
// Different commit → still upserted (not appended), attempts keeps climbing.
|
||||
recordSyncFailures([
|
||||
{ path: 'people/alice.md', error: 'YAML: unexpected colon in title' },
|
||||
], 'zzz999');
|
||||
expect(loadSyncFailures().length).toBe(3);
|
||||
const after = loadSyncFailures();
|
||||
expect(after.length).toBe(2);
|
||||
const alice2 = after.find(e => e.path === 'people/alice.md')!;
|
||||
expect(alice2.attempts).toBe(3);
|
||||
expect(alice2.commit).toBe('zzz999');
|
||||
});
|
||||
|
||||
test('acknowledgeSyncFailures marks unacked entries, leaves acked alone', async () => {
|
||||
@@ -129,6 +140,18 @@ describe('Bug 9 — doctor surfaces sync failures', () => {
|
||||
expect(source).toContain('unacknowledgedSyncFailures');
|
||||
expect(source).toContain("'gbrain sync --skip-failed'");
|
||||
});
|
||||
|
||||
// issue #1939: BOTH doctor surfaces (local buildChecks + remote
|
||||
// doctorReportRemote) must decide severity through the one shared helper so
|
||||
// they can never drift. The remote surface must no longer hand-roll an
|
||||
// `acknowledged_at` count (the old field-split that caused drift).
|
||||
test('both doctor surfaces use the shared decideSyncFailureSeverity', async () => {
|
||||
const source = await Bun.file(new URL('../src/commands/doctor.ts', import.meta.url)).text();
|
||||
const occurrences = source.split('decideSyncFailureSeverity').length - 1;
|
||||
expect(occurrences).toBeGreaterThanOrEqual(2); // local + remote
|
||||
// remote no longer counts `!entry.acknowledged_at` by hand.
|
||||
expect(source).not.toContain('if (!entry.acknowledged_at) unacked++');
|
||||
});
|
||||
});
|
||||
|
||||
describe('Bug 9 — sync.ts CLI flag wiring', () => {
|
||||
@@ -169,23 +192,25 @@ describe('Bug 9 — sync.ts CLI flag wiring', () => {
|
||||
expect(unacknowledgedSyncFailures().length).toBe(0);
|
||||
});
|
||||
|
||||
test('performSync gates sync.last_commit on failedFiles.length', async () => {
|
||||
test('performSync gates the bookmark through the shared failure ledger', async () => {
|
||||
const source = await Bun.file(new URL('../src/commands/sync.ts', import.meta.url)).text();
|
||||
// The gate exists and references the failure set.
|
||||
expect(source).toContain('failedFiles.length > 0');
|
||||
// issue #1939: the gate is now the shared applySyncFailureGate orchestrator.
|
||||
expect(source).toContain('applySyncFailureGate');
|
||||
expect(source).toContain('blocked_by_failures');
|
||||
});
|
||||
|
||||
test('performFullSync gates on result.failures from runImport', async () => {
|
||||
test('performFullSync routes failures through the same shared gate', async () => {
|
||||
const source = await Bun.file(new URL('../src/commands/sync.ts', import.meta.url)).text();
|
||||
expect(source).toContain('result.failures.length > 0');
|
||||
expect(source).toContain('result.failures');
|
||||
expect(source).toContain('applySyncFailureGate');
|
||||
});
|
||||
|
||||
test('runImport returns RunImportResult with failures list', async () => {
|
||||
test('runImport returns RunImportResult and records via the ledger', async () => {
|
||||
const source = await Bun.file(new URL('../src/commands/import.ts', import.meta.url)).text();
|
||||
expect(source).toContain('RunImportResult');
|
||||
expect(source).toContain('failures: Array<{ path: string; error: string }>');
|
||||
expect(source).toContain('recordSyncFailures');
|
||||
// issue #1939: import records source-scoped via recordFailures (sourceId, …).
|
||||
expect(source).toContain('recordFailures');
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user