From 2dc33fb865204c47098693da9dc7bd38d69a40f5 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Wed, 12 Aug 2026 11:21:16 -0700 Subject: [PATCH] fix(import): call clearFailures() for paths that succeed this run (#3843) Wave-assembled from PR #3843 by @bo-developing. Co-Authored-By: Bo --- src/commands/import.ts | 19 +++++ test/import-clear-failures-on-success.test.ts | 74 +++++++++++++++++++ 2 files changed, 93 insertions(+) create mode 100644 test/import-clear-failures-on-success.test.ts diff --git a/src/commands/import.ts b/src/commands/import.ts index e4fe8dc53..a0dba7223 100644 --- a/src/commands/import.ts +++ b/src/commands/import.ts @@ -289,6 +289,11 @@ export async function runImport( const importedSlugs: string[] = []; const errorCounts: Record = {}; const failures: Array<{ path: string; error: string }> = []; // Bug 9 + // #3839: paths that succeeded (imported OR unchanged) this run, keyed the + // same way as `failures` above (importRelPath) so a path that failed on a + // prior run and now succeeds clears its ledger row instead of staying + // `open` forever. + const succeededPaths: string[] = []; const startTime = Date.now(); // Progress on stderr so stdout stays clean for the final summary / --json payload. @@ -328,6 +333,7 @@ export async function runImport( importedSlugs.push(result.slug); // v0.33.2: path-based checkpoint — record only on success. completed.add(relativePath); + succeededPaths.push(importRelPath); // #3839 } else { skipped++; if (result.error && result.error !== 'unchanged') { @@ -340,6 +346,7 @@ export async function runImport( // 'unchanged' or no-error skip: content_hash matched a prior // successful import, so this file IS done for checkpoint purposes. completed.add(relativePath); + succeededPaths.push(importRelPath); // #3839 } } } catch (e: unknown) { @@ -570,6 +577,18 @@ export async function runImport( recordFailures(opts.sourceId ?? 'default', failures, gitHead); } + // #3839: a path that failed on a prior run and succeeded (imported or + // unchanged) this run must clear its ledger row — pre-fix, clearFailures + // existed but had no caller anywhere, so `open` rows never healed short + // of a manual `gbrain sync --skip-failed`. Runs on every non-empty + // success list regardless of whether this SAME run also had failures, + // so a stale row from an earlier run gets cleared even if today's run + // is only partially clean. + if (succeededPaths.length > 0) { + const { clearFailures } = await import('../core/sync.ts'); + clearFailures(opts.sourceId ?? 'default', succeededPaths); + } + // #2114 guard: the global sync.* keys describe THE brain repo (the // default source's working tree). Pre-fix this block rewrote them on // every git-repo import, silently repointing put_page write-through diff --git a/test/import-clear-failures-on-success.test.ts b/test/import-clear-failures-on-success.test.ts new file mode 100644 index 000000000..93e9cd94d --- /dev/null +++ b/test/import-clear-failures-on-success.test.ts @@ -0,0 +1,74 @@ +/** + * #3839 — `clearFailures()` (src/core/sync-failure-ledger.ts) existed and + * was unit-tested, but no command path ever called it. A path recorded as + * `open` in the sync-failures ledger stayed `open` forever, even after the + * exact same file imported cleanly on a later run — the ledger never + * self-healed short of a manual `gbrain sync --skip-failed`. + * + * Two-run scenario: a poison file fails run 1 (recorded), gets fixed, then + * succeeds run 2 (must clear). + * + * Hermetic PGLite in-memory. Sandboxes the failure ledger under a temp + * GBRAIN_HOME via `withEnv` so this test never touches the real + * ~/.gbrain/sync-failures.jsonl on the machine running it (see #2121). + */ + +import { describe, test, expect, beforeAll, afterAll } from 'bun:test'; +import { mkdtempSync, writeFileSync } from 'fs'; +import { execSync } from 'child_process'; +import { join } from 'path'; +import { tmpdir } from 'os'; +import { PGLiteEngine } from '../src/core/pglite-engine.ts'; +import { runImport } from '../src/commands/import.ts'; +import { withEnv } from './helpers/with-env.ts'; +import { loadSyncFailures } from '../src/core/sync-failure-ledger.ts'; + +let engine: PGLiteEngine; + +beforeAll(async () => { + engine = new PGLiteEngine(); + await engine.connect({}); + await engine.initSchema(); +}); + +afterAll(async () => { + await engine.disconnect(); +}); + +describe('a path that fails then succeeds clears its ledger row (#3839)', () => { + test('poison.md: open after run 1, gone after run 2', async () => { + const repo = mkdtempSync(join(tmpdir(), 'gbrain-clear-failures-')); + execSync('git init', { cwd: repo, stdio: 'pipe' }); + execSync('git config user.email "t@t.t"', { cwd: repo, stdio: 'pipe' }); + execSync('git config user.name "T"', { cwd: repo, stdio: 'pipe' }); + writeFileSync(join(repo, 'seed.md'), '---\ntype: note\n---\n# Seed\n\nbody\n'); + execSync('git add seed.md', { cwd: repo, stdio: 'pipe' }); + execSync('git commit -m seed', { cwd: repo, stdio: 'pipe' }); + + // Run 1: poison.md is oversized (deterministic soft-failure, same + // MAX_FILE_SIZE trigger as the #3838 test — no thrown exception needed). + const poisonPath = join(repo, 'poison.md'); + writeFileSync(poisonPath, '---\ntype: note\n---\n' + 'x'.repeat(5_000_001)); + + const gbrainHome = mkdtempSync(join(tmpdir(), 'gbrain-home-')); + + await withEnv({ GBRAIN_HOME: gbrainHome }, async () => { + await runImport(engine, [repo, '--fresh', '--no-embed', '--json']); + + const afterRun1 = loadSyncFailures().filter((f) => f.path === 'poison.md'); + expect(afterRun1.length).toBe(1); + expect(afterRun1[0].state).toBe('open'); + }); + + // Fix poison.md: small enough to import cleanly. Different content, so + // this is a real re-import, not a content_hash 'unchanged' skip. + writeFileSync(poisonPath, '---\ntype: note\n---\n# Poison, fixed\n\nnow well under the limit.\n'); + + await withEnv({ GBRAIN_HOME: gbrainHome }, async () => { + await runImport(engine, [repo, '--fresh', '--no-embed', '--json']); + + const afterRun2 = loadSyncFailures().filter((f) => f.path === 'poison.md'); + expect(afterRun2.length).toBe(0); + }); + }); +});