fix(import): make checkpoints staging-first — canonical dir identity + self-describing metadata

Port of #1731 (diazMelgarejo) onto current master. gbrain import wrote
~/.gbrain/import-checkpoint.json with the caller's raw dir argument, so
a checkpoint left behind by an interrupted run (e.g. SIGTERM) could carry
"." or a symlinked spelling — an identity that resolves to whatever CWD
the next consumer happens to run from. Downstream tooling that treated
the checkpoint dir as an owned staging boundary could then act on the
wrong directory.

- runImport captures the import target ONCE via resolveImportTargetDir
  (resolve + realpathSync) and threads that canonical value through
  collection, checkpoint load/save, and resume filtering
- checkpoints are self-describing (schema_version: 1, owner: "gbrain",
  kind: "import"); loadCheckpoint tolerates absent metadata (legacy
  path-based files) but rejects present-and-wrong metadata and any
  relative dir
- checkpoint contract documented in docs/guides/live-sync.md (llms
  bundle regenerated)
- test/import-resume.test.ts fixture now realpaths its tmpdir so planted
  checkpoints match the canonicalized dir (macOS /var -> /private/var)

Fixes #1728

Co-authored-by: Lawrence Melgarejo <Lawrence@cyre.me>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Sinabina
2026-07-17 14:09:17 -07:00
co-authored by Lawrence Melgarejo Claude Fable 5
parent 3aeb622dc7
commit e2edbc64ac
6 changed files with 166 additions and 6 deletions
+10
View File
@@ -131,6 +131,16 @@ hashes match. If both a cron and `--watch` fire simultaneously, no conflict.
history rewrite still hard-blocks even with `--skip-failed`. Run
`gbrain sync --skip-failed` to acknowledge a known-bad set yourself.
5. **Import checkpoints name the import target, not the caller's CWD.**
Interrupted `gbrain import <dir>` runs may leave
`~/.gbrain/import-checkpoint.json` so the next import can resume. The
checkpoint `dir` is the absolute, resolved import target captured when
import starts. It is not a cleanup instruction and it must not be
re-derived from the process working directory. Checkpoints written by
gbrain include `schema_version: 1`, `owner: "gbrain"`, and
`kind: "import"` so downstream tools can validate the contract before
deciding whether to resume.
## How to Verify
1. **Edit a file and search for the change.** Edit a brain markdown file,
+10
View File
@@ -2767,6 +2767,16 @@ hashes match. If both a cron and `--watch` fire simultaneously, no conflict.
history rewrite still hard-blocks even with `--skip-failed`. Run
`gbrain sync --skip-failed` to acknowledge a known-bad set yourself.
5. **Import checkpoints name the import target, not the caller's CWD.**
Interrupted `gbrain import <dir>` runs may leave
`~/.gbrain/import-checkpoint.json` so the next import can resume. The
checkpoint `dir` is the absolute, resolved import target captured when
import starts. It is not a cleanup instruction and it must not be
re-derived from the process working directory. Checkpoints written by
gbrain include `schema_version: 1`, `owner: "gbrain"`, and
`kind: "import"` so downstream tools can validate the contract before
deciding whether to resume.
## How to Verify
1. **Edit a file and search for the change.** Edit a brain markdown file,
+17 -1
View File
@@ -20,6 +20,7 @@ import {
loadCheckpoint,
saveCheckpoint,
clearCheckpoint,
resolveImportTargetDir,
resumeFilter,
} from '../core/import-checkpoint.ts';
@@ -168,7 +169,19 @@ export async function runImport(
console.error('Usage: gbrain import <dir> [--no-embed] [--workers N] [--fresh] [--source-id <id>] [--json]');
process.exit(1);
}
const dir: string = dirArg; // narrowed; survives closure capture
// #1728: capture the import target ONCE as an absolute real path. Every
// downstream consumer of `dir` (collection, checkpoint load/save, resume
// filtering) sees the same canonical identity — never the caller's `.`/
// relative spelling, which would make the persisted checkpoint `dir`
// resolve against whatever CWD a later process happens to run from.
let dir: string;
try {
dir = resolveImportTargetDir(dirArg);
} catch (e) {
const msg = e instanceof Error ? e.message : String(e);
console.error(`Import target is not readable: ${dirArg} (${msg})`);
process.exit(1);
}
// v0.31.2: collect under the right strategy. Pre-fix this called
// collectMarkdownFiles unconditionally — code-strategy first sync
@@ -288,6 +301,9 @@ export async function runImport(
catch { /* non-fatal */ }
}
saveCheckpoint(checkpointPath, {
schema_version: 1,
owner: 'gbrain',
kind: 'import',
dir,
completedPaths: Array.from(completed),
timestamp: new Date().toISOString(),
+36 -2
View File
@@ -1,5 +1,5 @@
import { existsSync, readFileSync, writeFileSync, renameSync, unlinkSync } from 'fs';
import { relative, isAbsolute } from 'path';
import { existsSync, readFileSync, writeFileSync, renameSync, unlinkSync, realpathSync } from 'fs';
import { relative, isAbsolute, resolve } from 'path';
/**
* Path-based import checkpoint.
@@ -25,6 +25,12 @@ import { relative, isAbsolute } from 'path';
* enter the set.
*/
export interface ImportCheckpoint {
/** Checkpoint payload schema. v1 is path-based with explicit producer metadata. */
schema_version: 1;
/** Producer marker for downstream consumers that validate before acting. */
owner: 'gbrain';
/** Checkpoint kind. Prevents unrelated checkpoint files from being treated as import state. */
kind: 'import';
/** Absolute brain directory the checkpoint was created against. Mismatch on resume → discard. */
dir: string;
/**
@@ -37,6 +43,21 @@ export interface ImportCheckpoint {
}
const OLD_FORMAT_LOG = 'Older checkpoint format detected — re-walking (cheap via content_hash)';
export const IMPORT_CHECKPOINT_SCHEMA_VERSION = 1;
export const IMPORT_CHECKPOINT_OWNER = 'gbrain';
export const IMPORT_CHECKPOINT_KIND = 'import';
/**
* Capture the import target once at run start. `resolve()` removes caller
* spelling such as `.` or `../staging`; `realpathSync()` collapses symlinks
* and proves the target exists. The returned value is the only directory
* identity import checkpoints should ever persist (#1728 — a raw `.` here
* made the checkpoint `dir` resolve to whatever CWD the NEXT consumer ran
* from, which downstream tooling treated as an owned staging directory).
*/
export function resolveImportTargetDir(dir: string): string {
return realpathSync(resolve(dir));
}
/**
* Load a checkpoint and verify it's compatible with the current run.
@@ -72,11 +93,21 @@ export function loadCheckpoint(path: string, currentDir: string): ImportCheckpoi
}
if (typeof obj.dir !== 'string') return null;
if (!isAbsolute(obj.dir)) return null;
if (obj.dir !== currentDir) return null;
// Self-describing metadata (#1728): absent fields are tolerated (legacy
// path-based checkpoints predate them), but present-and-wrong means the
// file was written by something else — don't resume from it.
if (obj.schema_version !== undefined && obj.schema_version !== IMPORT_CHECKPOINT_SCHEMA_VERSION) return null;
if (obj.owner !== undefined && obj.owner !== IMPORT_CHECKPOINT_OWNER) return null;
if (obj.kind !== undefined && obj.kind !== IMPORT_CHECKPOINT_KIND) return null;
if (typeof obj.timestamp !== 'string') return null;
if (!obj.completedPaths.every((p): p is string => typeof p === 'string')) return null;
return {
schema_version: IMPORT_CHECKPOINT_SCHEMA_VERSION,
owner: IMPORT_CHECKPOINT_OWNER,
kind: IMPORT_CHECKPOINT_KIND,
dir: obj.dir,
completedPaths: obj.completedPaths,
timestamp: obj.timestamp,
@@ -98,6 +129,9 @@ export function saveCheckpoint(path: string, cp: ImportCheckpoint): void {
// Sort for stable serialization — keeps diffs across snapshots minimal
// and tests deterministic.
const payload: ImportCheckpoint = {
schema_version: IMPORT_CHECKPOINT_SCHEMA_VERSION,
owner: IMPORT_CHECKPOINT_OWNER,
kind: IMPORT_CHECKPOINT_KIND,
dir: cp.dir,
completedPaths: [...cp.completedPaths].sort(),
timestamp: cp.timestamp,
+89 -1
View File
@@ -1,5 +1,5 @@
import { describe, test, expect, beforeEach, afterEach } from 'bun:test';
import { writeFileSync, readFileSync, existsSync, mkdtempSync, rmSync } from 'fs';
import { writeFileSync, readFileSync, existsSync, mkdtempSync, rmSync, mkdirSync, symlinkSync, realpathSync } from 'fs';
import { tmpdir } from 'os';
import { join } from 'path';
import {
@@ -7,6 +7,7 @@ import {
saveCheckpoint,
resumeFilter,
clearCheckpoint,
resolveImportTargetDir,
type ImportCheckpoint,
} from '../src/core/import-checkpoint.ts';
@@ -52,6 +53,9 @@ describe('loadCheckpoint', () => {
test('returns null when dir mismatches the current run', () => {
const cp: ImportCheckpoint = {
schema_version: 1,
owner: 'gbrain',
kind: 'import',
dir: '/other/brain',
completedPaths: ['a.md'],
timestamp: '2026-05-14T00:00:00Z',
@@ -86,6 +90,30 @@ describe('loadCheckpoint', () => {
expect(stderrCaptured).not.toContain('Older checkpoint format');
});
test('returns null when dir is relative (#1728 — CWD-dependent identity)', () => {
writeFileSync(cpPath, JSON.stringify({
schema_version: 1,
owner: 'gbrain',
kind: 'import',
dir: '.',
completedPaths: ['a.md'],
timestamp: '2026-01-01T00:00:00Z',
}));
expect(loadCheckpoint(cpPath, '.')).toBeNull();
});
test('returns null when self-describing metadata is wrong', () => {
writeFileSync(cpPath, JSON.stringify({
schema_version: 99,
owner: 'some-tool',
kind: 'other',
dir: '/tmp/example-brain',
completedPaths: ['a.md'],
timestamp: '2026-01-01T00:00:00Z',
}));
expect(loadCheckpoint(cpPath, '/tmp/example-brain')).toBeNull();
});
test('returns null when completedPaths contains non-strings', () => {
writeFileSync(cpPath, JSON.stringify({
dir: '/tmp/example-brain',
@@ -97,6 +125,9 @@ describe('loadCheckpoint', () => {
test('returns the checkpoint for valid v0.33.2 payload', () => {
const cp: ImportCheckpoint = {
schema_version: 1,
owner: 'gbrain',
kind: 'import',
dir: '/tmp/example-brain',
completedPaths: ['meetings/2026-05-13.md', 'concepts/foo.md'],
timestamp: '2026-05-14T12:34:56Z',
@@ -107,12 +138,31 @@ describe('loadCheckpoint', () => {
expect(loaded?.dir).toBe('/tmp/example-brain');
expect(loaded?.completedPaths).toEqual(['meetings/2026-05-13.md', 'concepts/foo.md']);
expect(loaded?.timestamp).toBe('2026-05-14T12:34:56Z');
expect(loaded?.schema_version).toBe(1);
expect(loaded?.owner).toBe('gbrain');
expect(loaded?.kind).toBe('import');
});
test('returns legacy path-based checkpoint without metadata as v1 in memory', () => {
writeFileSync(cpPath, JSON.stringify({
dir: '/tmp/example-brain',
completedPaths: ['a.md'],
timestamp: '2026-05-14T12:34:56Z',
}));
const loaded = loadCheckpoint(cpPath, '/tmp/example-brain');
expect(loaded?.schema_version).toBe(1);
expect(loaded?.owner).toBe('gbrain');
expect(loaded?.kind).toBe('import');
expect(loaded?.dir).toBe('/tmp/example-brain');
});
});
describe('saveCheckpoint', () => {
test('round-trips through loadCheckpoint', () => {
const cp: ImportCheckpoint = {
schema_version: 1,
owner: 'gbrain',
kind: 'import',
dir: '/tmp/example-brain',
completedPaths: ['a.md', 'b.md', 'c.md'],
timestamp: '2026-05-14T00:00:00Z',
@@ -125,16 +175,25 @@ describe('saveCheckpoint', () => {
test('serializes completedPaths sorted (deterministic output)', () => {
saveCheckpoint(cpPath, {
schema_version: 1,
owner: 'gbrain',
kind: 'import',
dir: '/tmp/example-brain',
completedPaths: ['z.md', 'a.md', 'm.md'],
timestamp: '2026-05-14T00:00:00Z',
});
const onDisk = JSON.parse(readFileSync(cpPath, 'utf-8'));
expect(onDisk.schema_version).toBe(1);
expect(onDisk.owner).toBe('gbrain');
expect(onDisk.kind).toBe('import');
expect(onDisk.completedPaths).toEqual(['a.md', 'm.md', 'z.md']);
});
test('atomic-ish write — no stray .tmp file after success', () => {
saveCheckpoint(cpPath, {
schema_version: 1,
owner: 'gbrain',
kind: 'import',
dir: '/tmp/example-brain',
completedPaths: ['a.md'],
timestamp: '2026-05-14T00:00:00Z',
@@ -148,6 +207,9 @@ describe('saveCheckpoint', () => {
const badPath = join(workDir, 'does-not-exist', 'cp.json');
expect(() =>
saveCheckpoint(badPath, {
schema_version: 1,
owner: 'gbrain',
kind: 'import',
dir: '/tmp/example-brain',
completedPaths: ['a.md'],
timestamp: '2026-05-14T00:00:00Z',
@@ -157,6 +219,32 @@ describe('saveCheckpoint', () => {
});
});
describe('resolveImportTargetDir', () => {
test('captures a relative import target as an absolute real path', () => {
const target = join(workDir, 'staging');
mkdirSync(target);
const cwd = process.cwd();
try {
process.chdir(workDir);
expect(resolveImportTargetDir('staging')).toBe(realpathSync(target));
} finally {
process.chdir(cwd);
}
});
test('collapses symlink spelling to the real import target', () => {
const target = join(workDir, 'real-staging');
const link = join(workDir, 'linked-staging');
mkdirSync(target);
symlinkSync(target, link);
expect(resolveImportTargetDir(link)).toBe(realpathSync(target));
});
test('throws when the target does not exist', () => {
expect(() => resolveImportTargetDir(join(workDir, 'nope'))).toThrow();
});
});
describe('resumeFilter', () => {
test('empty completed set returns all files unchanged', () => {
const all = ['a.md', 'b.md', 'c.md'];
+4 -2
View File
@@ -20,7 +20,7 @@
* `afterAll`) per CLAUDE.md test-isolation rules R3 + R4.
*/
import { describe, test, expect, beforeAll, afterAll, beforeEach, afterEach } from 'bun:test';
import { mkdtempSync, writeFileSync, readFileSync, existsSync, rmSync, mkdirSync } from 'fs';
import { mkdtempSync, writeFileSync, readFileSync, existsSync, rmSync, mkdirSync, realpathSync } from 'fs';
import { tmpdir } from 'os';
import { join } from 'path';
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
@@ -52,7 +52,9 @@ beforeEach(async () => {
gbrainHomeDir = join(workspace, '.gbrain');
mkdirSync(gbrainHomeDir, { recursive: true });
cpPath = join(gbrainHomeDir, 'import-checkpoint.json');
brainDir = mkdtempSync(join(tmpdir(), 'gbrain-import-resume-brain-'));
// #1728: realpath so planted checkpoints match runImport's canonicalized
// dir (macOS tmpdir is a /var → /private/var symlink).
brainDir = realpathSync(mkdtempSync(join(tmpdir(), 'gbrain-import-resume-brain-')));
});
afterEach(() => {