mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 22:15:33 +00:00
* feat: structured error code summary for sync --skip-failed (#500) When sync encounters per-file failures, the blocked/skip-failed messages now include a breakdown by error code (SLUG_MISMATCH, YAML_PARSE, etc.) instead of just a raw count. This makes it immediately obvious *why* files failed without requiring manual investigation. Changes: - Add classifyErrorCode() — maps error messages to ParseValidationCode - Add summarizeFailuresByCode() — groups failures into sorted code summary - SyncFailure now carries a 'code' field (backfilled on acknowledge) - acknowledgeSyncFailures() returns AcknowledgeResult {count, summary} - sync blocked + skip-failed messages show code breakdown - doctor sync_failures check shows code breakdown for both unacked and historical - 12 new tests for classifyErrorCode, summarizeFailuresByCode, and structured returns Before: Sync blocked: 2688 file(s) failed to parse. After: Sync blocked: 2688 file(s) failed to parse: SLUG_MISMATCH: 2685 YAML_DUPLICATE_KEY: 3 Closes #500 * fix: eng-review fixes for sync error-code classification - Reorder classifyErrorCode() so DB-layer errors (DB_DUPLICATE_KEY, STATEMENT_TIMEOUT) check BEFORE YAML patterns. Postgres "duplicate key value violates unique constraint" no longer mislabels as YAML_DUPLICATE_KEY. - Rewrite MISSING_OPEN/MISSING_CLOSE/EMPTY_FRONTMATTER/NULL_BYTES/NESTED_QUOTES regexes to match the canonical messages emitted by collectValidationErrors() in src/core/markdown.ts. Previous patterns (e.g. /missing.*open/i) never fired because the upstream throw site emits prose ("File is empty...", "No closing --- delimiter found"), not the code name. - Extract formatCodeBreakdown() helper that accepts either raw failures or pre-summarized {code, count}[] input. Replaces 3 duplicate inline builders in src/commands/sync.ts. - 15 new tests (37/37 pass on test/sync-failures.test.ts): - DB vs YAML duplicate-key disambiguation (3 cases) - Canonical-message coverage for the 5 frontmatter codes (7 cases) - acknowledgeSyncFailures() legacy-entry backfill branch (2 cases) - formatCodeBreakdown() dual-input shape (3 cases) - TODOS.md: file 3 follow-ups (P2 plumb structured ParseValidationCode; P0-at-ship CHANGELOG migration note for AcknowledgeResult; P3 concurrent- safe ack of sync-failures.jsonl). Eng-review plan: ~/.claude/plans/then-codex-synchronous-toucan.md Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: bump version and changelog (v0.22.9) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: 16-core runner + 4-way matrix shard for test job The unit test suite ran 22m17s on ubuntu-latest (2-core/7GB) because: - 187 test files run with bun test parallelism bounded by core count - 23 of those files spin up a fresh PGLiteEngine + initSchema in beforeEach, paying ~22s WASM cold-start per test on the small runner This commit fixes the runner side: - runs-on: ubuntu-latest-16-cores (16 vCPU / 64 GB RAM) - strategy.matrix.shard splits 4 parallel jobs, each running ~40 of 158 unit test files. Single-file wall-time floor is ~3 min after the test refactor, so 4 shards × 16 cores hits the floor quickly without wasting cores past it. - pre-test gates (typecheck, check-jsonb, check-progress, check-wasm) only run on shard 1 — they're not test files and don't benefit from sharding. scripts/test-shard.sh partitions test files by stable FNV-1a hash mod N. Same file always lands in the same shard, so retries are reproducible. Pure shell, portable to bash 3.2 (macOS) and bash 5.x (CI). Excludes test/e2e/ which runs via bun run test:e2e separately and needs DATABASE_URL. Also: ignore .claude/ harness state files (scheduled_tasks.lock etc) instead of just .claude/skills/. Cost: ~$0.19/run vs $0 (public repo, default runner is free). At 50 PRs/month that's ~$10/month for ~5x faster CI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: refactor top-3 PGLite-heavy files to share one engine per file Three test files were spinning up a fresh PGLiteEngine + connect + initSchema in beforeEach. PGLite WASM cold-start is ~22s on the small CI runner; doing this per test multiplied wall-time across the suite. The 3 files alone accounted for ~6.5 min of the 22m CI run (177s + 132s + 87s). Refactor: move PGLite setup to beforeAll (one engine per file), wipe data in beforeEach via the new test/helpers/reset-pglite.ts helper. The reset helper: - TRUNCATEs every public table CASCADE, including sources (so tests that register their own sources don't leak rows into the next test). - Re-seeds the default source row that pages.source_id's DEFAULT FKs against. Without this, the next page insert would fail FK validation. - Preserves schema_version so migration helpers don't think the brain is on v0. Files refactored: - test/extract-incremental.test.ts (8 tests, was 177s on CI) - test/brain-writer.test.ts (16 tests; only the scanBrainSources block uses PGLite, was 132s on CI) - test/sync.test.ts (37 tests; only the performSync dry-run block uses PGLite, was 87s on CI) All 61 tests still pass locally. The remaining 20 PGLite-heavy files use the same beforeEach anti-pattern; this commit only refactors the proven worst offenders. Sweep the rest in a follow-up if CI numbers indicate it's worth it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: fall back to ubuntu-latest for matrix shard The ubuntu-latest-16-cores label requires a provisioned larger-runner pool in repo/org settings. Without that setup, jobs queue indefinitely waiting for a runner that doesn't exist (verified: 4 shards stuck in 'queued' status with empty runner_name for 5+ min). Drop back to the default 2-core ubuntu-latest. The 4-way matrix shard still delivers ~5-6x speedup via parallelism alone — 4 jobs running in parallel, each handling ~40 of 158 unit test files. Cost stays $0 (default runner is free for public repos). If we ever provision a larger-runner pool, flip this label back to ubuntu-latest-16-cores. The matrix + sharder will use the bigger boxes unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Wintermute <wintermute@garrytan.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
384 lines
14 KiB
TypeScript
384 lines
14 KiB
TypeScript
import { describe, test, expect, beforeAll, afterAll, beforeEach, afterEach } from 'bun:test';
|
|
import { buildSyncManifest, isSyncable, pathToSlug } from '../src/core/sync.ts';
|
|
import { mkdtempSync, writeFileSync, rmSync, mkdirSync } from 'fs';
|
|
import { join } from 'path';
|
|
import { execSync } from 'child_process';
|
|
import { tmpdir } from 'os';
|
|
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
|
|
import { resetPgliteState } from './helpers/reset-pglite.ts';
|
|
|
|
describe('buildSyncManifest', () => {
|
|
test('parses A/M/D entries from single commit', () => {
|
|
const output = `A\tpeople/new-person.md\nM\tpeople/existing-person.md\nD\tpeople/deleted-person.md`;
|
|
const manifest = buildSyncManifest(output);
|
|
expect(manifest.added).toEqual(['people/new-person.md']);
|
|
expect(manifest.modified).toEqual(['people/existing-person.md']);
|
|
expect(manifest.deleted).toEqual(['people/deleted-person.md']);
|
|
expect(manifest.renamed).toEqual([]);
|
|
});
|
|
|
|
test('parses R100 rename entries', () => {
|
|
const output = `R100\tpeople/old-name.md\tpeople/new-name.md`;
|
|
const manifest = buildSyncManifest(output);
|
|
expect(manifest.renamed).toEqual([{ from: 'people/old-name.md', to: 'people/new-name.md' }]);
|
|
expect(manifest.added).toEqual([]);
|
|
expect(manifest.modified).toEqual([]);
|
|
expect(manifest.deleted).toEqual([]);
|
|
});
|
|
|
|
test('parses partial rename (R075)', () => {
|
|
const output = `R075\tpeople/old.md\tpeople/new.md`;
|
|
const manifest = buildSyncManifest(output);
|
|
expect(manifest.renamed).toEqual([{ from: 'people/old.md', to: 'people/new.md' }]);
|
|
});
|
|
|
|
test('handles empty diff', () => {
|
|
const manifest = buildSyncManifest('');
|
|
expect(manifest.added).toEqual([]);
|
|
expect(manifest.modified).toEqual([]);
|
|
expect(manifest.deleted).toEqual([]);
|
|
expect(manifest.renamed).toEqual([]);
|
|
});
|
|
|
|
test('handles mixed entries with blank lines', () => {
|
|
const output = `A\tpeople/a.md\n\nM\tpeople/b.md\n\nD\tpeople/c.md`;
|
|
const manifest = buildSyncManifest(output);
|
|
expect(manifest.added).toEqual(['people/a.md']);
|
|
expect(manifest.modified).toEqual(['people/b.md']);
|
|
expect(manifest.deleted).toEqual(['people/c.md']);
|
|
});
|
|
|
|
test('skips malformed lines', () => {
|
|
const output = `A\tpeople/a.md\ngarbage line\nM\tpeople/b.md`;
|
|
const manifest = buildSyncManifest(output);
|
|
expect(manifest.added).toEqual(['people/a.md']);
|
|
expect(manifest.modified).toEqual(['people/b.md']);
|
|
});
|
|
});
|
|
|
|
describe('isSyncable', () => {
|
|
test('accepts normal .md files', () => {
|
|
expect(isSyncable('people/pedro-franceschi.md')).toBe(true);
|
|
expect(isSyncable('meetings/2026-04-03-lunch.md')).toBe(true);
|
|
expect(isSyncable('daily/2026-04-05.md')).toBe(true);
|
|
expect(isSyncable('notes.md')).toBe(true);
|
|
});
|
|
|
|
test('accepts .mdx files', () => {
|
|
expect(isSyncable('components/hero.mdx')).toBe(true);
|
|
expect(isSyncable('docs/getting-started.mdx')).toBe(true);
|
|
});
|
|
|
|
test('rejects non-.md/.mdx files', () => {
|
|
expect(isSyncable('people/photo.jpg')).toBe(false);
|
|
expect(isSyncable('config.json')).toBe(false);
|
|
expect(isSyncable('src/cli.ts')).toBe(false);
|
|
});
|
|
|
|
test('rejects files in hidden directories', () => {
|
|
expect(isSyncable('.git/config')).toBe(false);
|
|
expect(isSyncable('.obsidian/plugins.md')).toBe(false);
|
|
expect(isSyncable('people/.hidden/secret.md')).toBe(false);
|
|
});
|
|
|
|
test('rejects .raw/ sidecar directories', () => {
|
|
expect(isSyncable('people/pedro.raw/source.md')).toBe(false);
|
|
expect(isSyncable('dir/.raw/notes.md')).toBe(false);
|
|
});
|
|
|
|
test('rejects skip-list basenames', () => {
|
|
expect(isSyncable('schema.md')).toBe(false);
|
|
expect(isSyncable('index.md')).toBe(false);
|
|
expect(isSyncable('log.md')).toBe(false);
|
|
expect(isSyncable('README.md')).toBe(false);
|
|
expect(isSyncable('people/README.md')).toBe(false);
|
|
});
|
|
|
|
test('rejects ops/ directory', () => {
|
|
expect(isSyncable('ops/deploy-log.md')).toBe(false);
|
|
expect(isSyncable('ops/config.md')).toBe(false);
|
|
});
|
|
});
|
|
|
|
describe('pathToSlug', () => {
|
|
test('strips .md extension and lowercases', () => {
|
|
expect(pathToSlug('people/pedro-franceschi.md')).toBe('people/pedro-franceschi');
|
|
});
|
|
|
|
test('normalizes to lowercase', () => {
|
|
expect(pathToSlug('People/Pedro-Franceschi.md')).toBe('people/pedro-franceschi');
|
|
});
|
|
|
|
test('strips leading slash', () => {
|
|
expect(pathToSlug('/people/pedro.md')).toBe('people/pedro');
|
|
});
|
|
|
|
test('normalizes backslash separators', () => {
|
|
expect(pathToSlug('people\\pedro.md')).toBe('people/pedro');
|
|
});
|
|
|
|
test('handles flat files', () => {
|
|
expect(pathToSlug('notes.md')).toBe('notes');
|
|
});
|
|
|
|
test('handles nested paths', () => {
|
|
expect(pathToSlug('projects/gbrain/spec.md')).toBe('projects/gbrain/spec');
|
|
});
|
|
|
|
test('adds repo prefix when provided', () => {
|
|
expect(pathToSlug('people/pedro.md', 'brain')).toBe('brain/people/pedro');
|
|
});
|
|
|
|
test('no prefix when not provided', () => {
|
|
expect(pathToSlug('people/pedro.md')).toBe('people/pedro');
|
|
});
|
|
|
|
test('handles empty string', () => {
|
|
expect(pathToSlug('')).toBe('');
|
|
});
|
|
|
|
test('handles file with only extension', () => {
|
|
expect(pathToSlug('.md')).toBe('');
|
|
});
|
|
|
|
test('slugifies spaces to hyphens', () => {
|
|
expect(pathToSlug('Apple Notes/2017-05-03 ohmygreen.md')).toBe('apple-notes/2017-05-03-ohmygreen');
|
|
});
|
|
|
|
test('strips special characters', () => {
|
|
expect(pathToSlug('notes/meeting (march 2024).md')).toBe('notes/meeting-march-2024');
|
|
});
|
|
});
|
|
|
|
describe('isSyncable edge cases', () => {
|
|
test('rejects uppercase .MD extension', () => {
|
|
// isSyncable checks path.endsWith('.md'), so .MD should fail
|
|
expect(isSyncable('people/someone.MD')).toBe(false);
|
|
});
|
|
|
|
test('rejects files with no extension', () => {
|
|
expect(isSyncable('README')).toBe(false);
|
|
});
|
|
|
|
test('accepts deeply nested .md files', () => {
|
|
expect(isSyncable('a/b/c/d/e/f/deep.md')).toBe(true);
|
|
});
|
|
|
|
test('rejects .md files inside nested hidden dirs', () => {
|
|
expect(isSyncable('docs/.internal/secret.md')).toBe(false);
|
|
});
|
|
});
|
|
|
|
describe('buildSyncManifest edge cases', () => {
|
|
test('handles tab-separated fields correctly', () => {
|
|
const output = "A\tpath/to/file.md";
|
|
const manifest = buildSyncManifest(output);
|
|
expect(manifest.added).toEqual(['path/to/file.md']);
|
|
});
|
|
|
|
test('handles multiple renames', () => {
|
|
const output = [
|
|
'R100\told/a.md\tnew/a.md',
|
|
'R095\told/b.md\tnew/b.md',
|
|
].join('\n');
|
|
const manifest = buildSyncManifest(output);
|
|
expect(manifest.renamed).toHaveLength(2);
|
|
expect(manifest.renamed[0].from).toBe('old/a.md');
|
|
expect(manifest.renamed[1].from).toBe('old/b.md');
|
|
});
|
|
|
|
test('ignores unknown status codes', () => {
|
|
const output = "X\tunknown/file.md";
|
|
const manifest = buildSyncManifest(output);
|
|
expect(manifest.added).toEqual([]);
|
|
expect(manifest.modified).toEqual([]);
|
|
expect(manifest.deleted).toEqual([]);
|
|
expect(manifest.renamed).toEqual([]);
|
|
});
|
|
});
|
|
|
|
// ────────────────────────────────────────────────────────────────
|
|
// performSync dry-run (v0.17 regression guard for full-sync silent writes)
|
|
// ────────────────────────────────────────────────────────────────
|
|
|
|
describe('performSync dry-run never writes', () => {
|
|
let engine: PGLiteEngine;
|
|
let repoPath: string;
|
|
|
|
// One PGLite per file — beforeEach wipes data only. Each test still gets a
|
|
// fresh git repo via mkdtempSync, but skips the ~20s PGLite cold-start.
|
|
beforeAll(async () => {
|
|
engine = new PGLiteEngine();
|
|
await engine.connect({});
|
|
await engine.initSchema();
|
|
});
|
|
|
|
afterAll(async () => {
|
|
await engine.disconnect();
|
|
});
|
|
|
|
beforeEach(async () => {
|
|
await resetPgliteState(engine);
|
|
repoPath = mkdtempSync(join(tmpdir(), 'gbrain-sync-dryrun-'));
|
|
execSync('git init', { cwd: repoPath, stdio: 'pipe' });
|
|
execSync('git config user.email "test@test.com"', { cwd: repoPath, stdio: 'pipe' });
|
|
execSync('git config user.name "Test"', { cwd: repoPath, stdio: 'pipe' });
|
|
mkdirSync(join(repoPath, 'people'), { recursive: true });
|
|
writeFileSync(join(repoPath, 'people/alice.md'), [
|
|
'---',
|
|
'type: person',
|
|
'title: Alice',
|
|
'---',
|
|
'',
|
|
'Alice is a person.',
|
|
].join('\n'));
|
|
writeFileSync(join(repoPath, 'people/bob.md'), [
|
|
'---',
|
|
'type: person',
|
|
'title: Bob',
|
|
'---',
|
|
'',
|
|
'Bob is another person.',
|
|
].join('\n'));
|
|
execSync('git add -A && git commit -m "initial"', { cwd: repoPath, stdio: 'pipe' });
|
|
});
|
|
|
|
afterEach(() => {
|
|
if (repoPath) rmSync(repoPath, { recursive: true, force: true });
|
|
});
|
|
|
|
test('first-sync dry-run does NOT write to DB or advance the bookmark', async () => {
|
|
const { performSync } = await import('../src/commands/sync.ts');
|
|
const result = await performSync(engine, {
|
|
repoPath,
|
|
dryRun: true,
|
|
noPull: true,
|
|
noEmbed: true,
|
|
});
|
|
|
|
// Status + counts reflect what WOULD be imported.
|
|
expect(result.status).toBe('dry_run');
|
|
expect(result.added).toBe(2); // alice + bob, both syncable
|
|
expect(result.chunksCreated).toBe(0);
|
|
expect(result.embedded).toBe(0);
|
|
|
|
// DB is clean: no pages written.
|
|
expect(await engine.getPage('people/alice')).toBeNull();
|
|
expect(await engine.getPage('people/bob')).toBeNull();
|
|
|
|
// Bookmark NOT set — this is the regression the guard enforces.
|
|
expect(await engine.getConfig('sync.last_commit')).toBeNull();
|
|
expect(await engine.getConfig('sync.repo_path')).toBeNull();
|
|
});
|
|
|
|
test('incremental dry-run does NOT write to DB or advance the bookmark', async () => {
|
|
const { performSync } = await import('../src/commands/sync.ts');
|
|
// First do a real sync to seed the bookmark.
|
|
const real = await performSync(engine, {
|
|
repoPath,
|
|
noPull: true,
|
|
noEmbed: true,
|
|
});
|
|
expect(real.status).toBe('first_sync');
|
|
const bookmarkAfterReal = await engine.getConfig('sync.last_commit');
|
|
expect(bookmarkAfterReal).not.toBeNull();
|
|
|
|
// Add a third file.
|
|
writeFileSync(join(repoPath, 'people/carol.md'), [
|
|
'---',
|
|
'type: person',
|
|
'title: Carol',
|
|
'---',
|
|
'',
|
|
'Carol joins the cast.',
|
|
].join('\n'));
|
|
execSync('git add -A && git commit -m "add carol"', { cwd: repoPath, stdio: 'pipe' });
|
|
|
|
// Incremental sync in dry-run mode.
|
|
const result = await performSync(engine, {
|
|
repoPath,
|
|
dryRun: true,
|
|
noPull: true,
|
|
noEmbed: true,
|
|
});
|
|
|
|
expect(result.status).toBe('dry_run');
|
|
expect(result.added).toBe(1); // carol only
|
|
expect(result.chunksCreated).toBe(0);
|
|
expect(result.embedded).toBe(0);
|
|
|
|
// carol is NOT in the DB.
|
|
expect(await engine.getPage('people/carol')).toBeNull();
|
|
// alice + bob still present from the real sync.
|
|
expect(await engine.getPage('people/alice')).not.toBeNull();
|
|
expect(await engine.getPage('people/bob')).not.toBeNull();
|
|
|
|
// Bookmark unchanged — still at the pre-carol commit.
|
|
const bookmarkAfterDry = await engine.getConfig('sync.last_commit');
|
|
expect(bookmarkAfterDry).toBe(bookmarkAfterReal);
|
|
});
|
|
|
|
test('full-sync (--full) dry-run does NOT write to DB or advance the bookmark', async () => {
|
|
const { performSync } = await import('../src/commands/sync.ts');
|
|
// Seed the bookmark so we hit the full-sync-with-bookmark path when --full is set.
|
|
await performSync(engine, { repoPath, noPull: true, noEmbed: true });
|
|
// Clear DB so we can observe that a --full dry-run doesn't re-import.
|
|
await (engine as any).db.exec(`DELETE FROM content_chunks; DELETE FROM pages;`);
|
|
const bookmarkBefore = await engine.getConfig('sync.last_commit');
|
|
expect(bookmarkBefore).not.toBeNull();
|
|
|
|
const result = await performSync(engine, {
|
|
repoPath,
|
|
full: true, // force full-sync path
|
|
dryRun: true,
|
|
noPull: true,
|
|
noEmbed: true,
|
|
});
|
|
|
|
expect(result.status).toBe('dry_run');
|
|
expect(result.added).toBe(2); // alice + bob would be imported
|
|
expect(result.chunksCreated).toBe(0);
|
|
|
|
// DB empty — full-sync dry-run did not reimport.
|
|
expect(await engine.getPage('people/alice')).toBeNull();
|
|
expect(await engine.getPage('people/bob')).toBeNull();
|
|
|
|
// Bookmark unchanged.
|
|
const bookmarkAfter = await engine.getConfig('sync.last_commit');
|
|
expect(bookmarkAfter).toBe(bookmarkBefore);
|
|
});
|
|
|
|
test('SyncResult exposes embedded count field', async () => {
|
|
const { performSync } = await import('../src/commands/sync.ts');
|
|
const result = await performSync(engine, {
|
|
repoPath,
|
|
dryRun: true,
|
|
noPull: true,
|
|
noEmbed: true,
|
|
});
|
|
// Structural assertion: the contract includes `embedded: number`.
|
|
expect(typeof result.embedded).toBe('number');
|
|
});
|
|
});
|
|
|
|
describe('sync regression — #132 nested transaction deadlock', () => {
|
|
test('src/commands/sync.ts does not wrap the add/modify loop in engine.transaction()', async () => {
|
|
const source = await Bun.file(new URL('../src/commands/sync.ts', import.meta.url)).text();
|
|
// Accept either of the historical loop shapes: the original inline
|
|
// `for (const path of [...filtered.added, ...filtered.modified])` or
|
|
// the v0.15.2 progress-wrapped variant where the list is hoisted into
|
|
// a local `addsAndMods` variable first.
|
|
const inlineIdx = source.indexOf('for (const path of [...filtered.added, ...filtered.modified]');
|
|
const hoistedIdx = source.indexOf('const addsAndMods = [...filtered.added, ...filtered.modified]');
|
|
const loopStart = inlineIdx !== -1 ? inlineIdx : hoistedIdx;
|
|
expect(loopStart).toBeGreaterThan(-1);
|
|
const prelude = source.slice(0, loopStart);
|
|
const lastTxIdx = prelude.lastIndexOf('engine.transaction');
|
|
if (lastTxIdx !== -1) {
|
|
const lineStart = prelude.lastIndexOf('\n', lastTxIdx) + 1;
|
|
const line = prelude.slice(lineStart, prelude.indexOf('\n', lastTxIdx));
|
|
expect(line.trim().startsWith('//')).toBe(true);
|
|
}
|
|
});
|
|
});
|