admin/gbrain
1
0
Fork 0
mirror of https://github.com/garrytan/gbrain.git synced 2026-08-14 00:48:18 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(sync): prevent dry-run pull and out-of-strategy page deletion (#3624)

Browse Source 
Co-Authored-By: Mammad M. <mammad@digit.az>
This commit is contained in:
Garry Tan
2026-08-01 10:06:17 +08:00
committed by Sina Matian
co-authored by Mammad M.
parent 61ef727710
commit c0b28f104d
2 changed files with 126 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/commands/sync.ts
+28 -28
View File
@@ -1980,7 +1980,7 @@ async function performSyncInner(engine: BrainEngine, opts: SyncOpts): Promise<Sy
serr(`[gbrain phase] sync.detect_head`);
// Detect detached HEAD up front so the working-tree fallback fires for both
// the default sync and `--no-pull` callers. Only the actual git pull is
// gated on opts.noPull.
// gated on opts.noPull or opts.dryRun.
const detachedHead = isDetachedHead(gitContextRoot);
if (detachedHead && !opts.noPull) {
// Print the caller's repoPath spelling (not the realpathed git root) —
@@ -1988,7 +1988,7 @@ async function performSyncInner(engine: BrainEngine, opts: SyncOpts): Promise<Sy
serr(`Detached HEAD on ${repoPath}; skipping git pull. Syncing from local working tree.`);
}
// Git pull (unless --no-pull). v0.28.1 codex finding (HIGH): the legacy
// Git pull (unless --no-pull or --dry-run). v0.28.1 codex finding (HIGH): the legacy
// git() helper at sync.ts:192 spawns git without GIT_SSRF_FLAGS, so
// every steady-state pull was bypassing the redirect/submodule/protocol
// hardening that cloneRepo applies. Route through pullRepo from
@@ -2032,7 +2032,7 @@ async function performSyncInner(engine: BrainEngine, opts: SyncOpts): Promise<Sy
// exited 0 with "Already up to date" and doctor's sync_freshness never
// fired because last_sync_at kept advancing.
let pullFailed = false;
if (!opts.noPull && !detachedHead && originRemotePresent) {
if (!opts.dryRun && !opts.noPull && !detachedHead && originRemotePresent) {
const _t0 = Date.now();
serr(`[gbrain phase] sync.git_pull start`);
try {
@@ -2391,6 +2391,31 @@ async function performSyncInner(engine: BrainEngine, opts: SyncOpts): Promise<Sy
}
}
const totalChanges = filtered.added.length + filtered.modified.length +
filtered.deleted.length + filtered.renamed.length;
// Dry run
if (opts.dryRun) {
slog(`Sync dry run: ${lastCommit.slice(0, 8)}..${headCommit.slice(0, 8)}`);
if (filtered.added.length) slog(` Added: ${filtered.added.join(', ')}`);
if (filtered.modified.length) slog(` Modified: ${filtered.modified.join(', ')}`);
if (filtered.deleted.length) slog(` Deleted: ${filtered.deleted.join(', ')}`);
if (filtered.renamed.length) slog(` Renamed: ${filtered.renamed.map(r => `${r.from} -> ${r.to}`).join(', ')}`);
if (totalChanges === 0) slog(` No syncable changes.`);
return {
status: 'dry_run',
fromCommit: lastCommit,
toCommit: headCommit,
added: filtered.added.length,
modified: filtered.modified.length,
deleted: filtered.deleted.length,
renamed: filtered.renamed.length,
chunksCreated: 0,
embedded: 0,
pagesAffected: [],
};
}
// Delete pages that became un-syncable (modified but filtered out).
// v0.20.0 Cathedral II SP-5: resolveSlugForPath picks the right slug shape
// (markdown vs code) based on the chunker's classifier, so a Rust file that
@@ -2438,31 +2463,6 @@ async function performSyncInner(engine: BrainEngine, opts: SyncOpts): Promise<Sy
} catch { /* ignore */ }
}
const totalChanges = filtered.added.length + filtered.modified.length +
filtered.deleted.length + filtered.renamed.length;
// Dry run
if (opts.dryRun) {
slog(`Sync dry run: ${lastCommit.slice(0, 8)}..${headCommit.slice(0, 8)}`);
if (filtered.added.length) slog(` Added: ${filtered.added.join(', ')}`);
if (filtered.modified.length) slog(` Modified: ${filtered.modified.join(', ')}`);
if (filtered.deleted.length) slog(` Deleted: ${filtered.deleted.join(', ')}`);
if (filtered.renamed.length) slog(` Renamed: ${filtered.renamed.map(r => `${r.from} -> ${r.to}`).join(', ')}`);
if (totalChanges === 0) slog(` No syncable changes.`);
return {
status: 'dry_run',
fromCommit: lastCommit,
toCommit: headCommit,
added: filtered.added.length,
modified: filtered.modified.length,
deleted: filtered.deleted.length,
renamed: filtered.renamed.length,
chunksCreated: 0,
embedded: 0,
pagesAffected: [],
};
}
if (totalChanges === 0) {
// #3068: same guard as the git-HEAD-equality gate above — a failed pull
// plus zero imports must not produce a clean `up_to_date` (and must not
test/sync.test.ts
+98
View File
@@ -422,6 +422,104 @@ describe('performSync dry-run never writes', () => {
expect(bookmarkAfterDry).toBe(bookmarkAfterReal);
});
test('strategy-changing dry-run preserves previously indexed out-of-strategy pages', async () => {
const { performSync } = await import('../src/commands/sync.ts');
await performSync(engine, {
repoPath,
noPull: true,
noEmbed: true,
});
const pageBefore = await engine.getPage('people/alice');
const bookmarkBefore = await engine.getConfig('sync.last_commit');
expect(pageBefore).not.toBeNull();
expect(bookmarkBefore).not.toBeNull();
writeFileSync(join(repoPath, 'people/alice.md'), [
'---',
'type: person',
'title: Alice',
'---',
'',
'Alice changed after the initial sync.',
].join('\n'));
execSync('git add -A && git commit -m "update alice"', { cwd: repoPath, stdio: 'pipe' });
const result = await performSync(engine, {
repoPath,
strategy: 'code',
dryRun: true,
noPull: true,
noEmbed: true,
});
expect(result.status).toBe('dry_run');
const pageAfter = await engine.getPage('people/alice');
expect(pageAfter).not.toBeNull();
expect(pageAfter!.compiled_truth).toBe(pageBefore!.compiled_truth);
expect(await engine.getConfig('sync.last_commit')).toBe(bookmarkBefore);
});
test('strategy-changing real sync deletes previously indexed out-of-strategy pages', async () => {
const { performSync } = await import('../src/commands/sync.ts');
await performSync(engine, {
repoPath,
noPull: true,
noEmbed: true,
});
expect(await engine.getPage('people/alice')).not.toBeNull();
writeFileSync(join(repoPath, 'people/alice.md'), [
'---',
'type: person',
'title: Alice',
'---',
'',
'Alice changed after the initial sync.',
].join('\n'));
execSync('git add -A && git commit -m "update alice"', { cwd: repoPath, stdio: 'pipe' });
await performSync(engine, {
repoPath,
strategy: 'code',
noPull: true,
noEmbed: true,
});
expect(await engine.getPage('people/alice')).toBeNull();
});
test('dry-run does not attempt git pull when origin exists', async () => {
const { performSync } = await import('../src/commands/sync.ts');
const remotePath = mkdtempSync(join(tmpdir(), 'gbrain-sync-dryrun-remote-'));
try {
execSync('git init --bare', { cwd: remotePath, stdio: 'pipe' });
execSync(`git remote add origin ${JSON.stringify(remotePath)}`, {
cwd: repoPath,
stdio: 'pipe',
});
const messages: string[] = [];
const originalError = console.error;
console.error = (...args: unknown[]) => {
messages.push(args.map(String).join(' '));
};
try {
const result = await performSync(engine, {
repoPath,
dryRun: true,
noEmbed: true,
});
expect(result.status).toBe('dry_run');
} finally {
console.error = originalError;
}
expect(messages.some(message => message.includes('sync.git_pull start'))).toBe(false);
expect(messages.some(message => message.includes('git pull failed'))).toBe(false);
} finally {
rmSync(remotePath, { recursive: true, force: true });
}
});
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.