mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-31 04:07:52 +00:00
fix(sync): stop the sync data-loss family — ops/ prune, DB-only write-through, full-sync gate drift (#2404, #2426, #2607) (#2938)
Three verified-open defects, one family: sync silently destroying or diverging on content it should preserve. #2404 (P0) — 'ops' was hardcoded in PRUNE_DIR_NAMES (a v0.2.0-era carve-out), so any path with an ops segment was 'pruned-dir': committed ops/*.md never imported, and modified ops/* files hit the unsyncableModified delete loop (whose #1433 guard only spared 'metafile'), silently deleting put-created pages like the bundled daily-task-manager's canonical ops/tasks on every sync. Fix: remove 'ops' from the prune list (ordinary user content; the vendor/generated entries stay), and harden the delete loop to also skip 'pruned-dir' — a page under a pruned dir can only exist via a deliberate put_page. #2426 (P0) — write-through content stayed DB-only and was deleted by sync --full. All three compounding bugs fixed: 1. writePageThrough now best-effort commits the artifact (path-limited git commit) on durability-hardened repos, so the post-commit hook can push it; result carries committed?: boolean. 2. scripts/brain-commit-push.sh stages+commits BEFORE any pull — the old fetch+pull-rebase-first order aborted on any dirty tree, so the helper could never commit a MODIFIED page; brain_push's rebase-on-reject already handles an advanced remote. 3. The full-sync delete-reconcile partitions stale pages by git history (listEverCommittedPaths): never-committed source_paths are DB-only write-through — pages are KEPT and re-exported to the working tree instead of soft-deleted. Builds on the #2828 mass-delete valve (covers the below-valve cases). #2607 — the sync --full git ls-files fast path bypassed pruneDir, so a full pass imported (and resurrected soft-deleted) pages under dot-dirs and vendored trees that incremental sync excludes. Fix: isCollectibleForWalker applies the same segment-level pruneDir gate as classifySync, so full and incremental enumeration agree. One regression test per defect (all verified failing against master src): test/sync-ops-pages.serial.test.ts, test/write-through-commit.serial.test.ts, the #2426 helper-order test in test/brain-durability-hook.serial.test.ts, test/sync-reconcile-db-only.serial.test.ts, test/import-git-fastpath-prune.test.ts. Fixes #2404 Fixes #2426 Fixes #2607 Co-authored-by: Sinabina <sinabina@Sinabinas-MacBook-Pro-4.local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Sinabina
Claude Fable 5
parent
a8e6b1d177
commit
42375bded5
@@ -188,7 +188,7 @@ export function walkMarkdownFiles(dir: string): { path: string; relPath: string
|
||||
// Pre-fix, this walker had only an ad-hoc dot-prefix exclusion and didn't
|
||||
// call isSyncable at all — so it descended into `node_modules/`, emitted
|
||||
// markdown files from there, AND ignored the canonical exclusion list
|
||||
// (`.raw/`, `ops/`, README.md, etc.). Now: pruneDir skips entire vendor
|
||||
// (`.raw/`, README.md, etc.). Now: pruneDir skips entire vendor
|
||||
// subtrees before recursion (saving IO), and isSyncable filters the emit
|
||||
// set against the canonical markdown-strategy rules.
|
||||
const files: { path: string; relPath: string }[] = [];
|
||||
|
||||
+12
-1
@@ -526,10 +526,21 @@ function isCollectibleForWalker(
|
||||
strategy: SyncStrategy,
|
||||
multimodalOn: boolean,
|
||||
): boolean {
|
||||
// #2607: apply the SAME segment-level prune gate as incremental sync's
|
||||
// `classifySync` (core/sync.ts). The FS walk below prunes at descent time,
|
||||
// but the git fast path enumerates via `git ls-files` and historically
|
||||
// filtered only by extension — so `sync --full` imported (and resurrected
|
||||
// previously-deleted) pages under dot-dirs / vendored trees that incremental
|
||||
// sync excludes. Full and incremental must agree on the exclusion set.
|
||||
// (In the FS-walk route `path` is a basename, so this is the same dot-file
|
||||
// check pruneDir already applied there — no behavior change on that route.)
|
||||
const segments = path.split('/');
|
||||
if (segments.some((seg) => !pruneDir(seg))) return false;
|
||||
|
||||
// Metafiles are directory scaffolding (READMEs / index / log / schema /
|
||||
// resolver), not typed brain pages — same exclusion `sync`'s `isSyncable`
|
||||
// applies. Guards both the FS-walk and the git-fast-path collection routes.
|
||||
const basename = path.split('/').pop() || '';
|
||||
const basename = segments[segments.length - 1] || '';
|
||||
if ((SYNC_SKIP_FILES as readonly string[]).includes(basename)) return false;
|
||||
|
||||
switch (strategy) {
|
||||
|
||||
+73
-3
@@ -1897,7 +1897,13 @@ async function performSyncInner(engine: BrainEngine, opts: SyncOpts): Promise<Sy
|
||||
const pageOpts = opts.sourceId ? { sourceId: opts.sourceId } : undefined;
|
||||
for (const path of unsyncableModified) {
|
||||
// v0.41.13 #1433: never delete on metafile classification.
|
||||
if (unsyncableReason(path, syncOpts) === 'metafile') continue;
|
||||
// #2404 hardening: same for 'pruned-dir' — a page under a pruned
|
||||
// directory can only exist via a deliberate put_page (sync never
|
||||
// imports those paths), so "the file was modified" is not evidence
|
||||
// the page is stale. Deleting here silently destroyed put-created
|
||||
// pages every time their materialized file landed in a commit.
|
||||
const reason = unsyncableReason(path, syncOpts);
|
||||
if (reason === 'metafile' || reason === 'pruned-dir') continue;
|
||||
const slug = await resolveSlugByPathOrSourcePath(engine, path, opts.sourceId);
|
||||
try {
|
||||
const existing = await engine.getPage(slug, pageOpts);
|
||||
@@ -3169,9 +3175,45 @@ async function performFullSync(
|
||||
`GBRAIN_ALLOW_MASS_RECONCILE=1 to restore the old behavior.`,
|
||||
);
|
||||
} else if (plan.staleSlugs.length > 0) {
|
||||
// #2426: a stale page whose source_path was NEVER committed to git is
|
||||
// DB-only write-through (the file was written into the clone but never
|
||||
// committed/pushed, then lost — e.g. a fresh clone). "Absent from git"
|
||||
// is the SYMPTOM of that bug, not evidence the content is disposable.
|
||||
// Keep those pages and re-export their markdown to the working tree so
|
||||
// they're file-backed again; only pages whose file once existed in git
|
||||
// history (i.e. was genuinely deleted) are reconcile-deleted.
|
||||
const everCommitted = listEverCommittedPaths(repoPath);
|
||||
const pathBySlug = new Map(rows.map(r => [r.slug, r.source_path]));
|
||||
let deletableSlugs = plan.staleSlugs;
|
||||
const dbOnlySlugs: string[] = [];
|
||||
if (everCommitted) {
|
||||
deletableSlugs = [];
|
||||
for (const slug of plan.staleSlugs) {
|
||||
const sp = pathBySlug.get(slug);
|
||||
if (sp && !everCommitted.has(sp.replace(/\\/g, '/'))) dbOnlySlugs.push(slug);
|
||||
else deletableSlugs.push(slug);
|
||||
}
|
||||
}
|
||||
if (dbOnlySlugs.length > 0) {
|
||||
let reExported = 0;
|
||||
try {
|
||||
const { writePageThrough } = await import('../core/write-through.ts');
|
||||
for (const slug of dbOnlySlugs) {
|
||||
const r = await writePageThrough(engine, slug, { sourceId: sid });
|
||||
if (r.written) reExported++;
|
||||
}
|
||||
} catch { /* best-effort — pages are preserved either way */ }
|
||||
serr(
|
||||
`\n Kept ${dbOnlySlugs.length} page(s) whose markdown was never committed to git ` +
|
||||
`(DB-only write-through — not deleting).` +
|
||||
(reExported > 0 ? ` Re-exported ${reExported} of them to the working tree.` : '') +
|
||||
`\n Commit + push them (e.g. scripts/brain-commit-push.sh, or 'gbrain sources harden') ` +
|
||||
`so the next sync sees them as file-backed.`,
|
||||
);
|
||||
}
|
||||
const deleteScopedOpts = { sourceId: sid };
|
||||
for (let i = 0; i < plan.staleSlugs.length; i += DELETE_BATCH_SIZE) {
|
||||
const batch = plan.staleSlugs.slice(i, i + DELETE_BATCH_SIZE);
|
||||
for (let i = 0; i < deletableSlugs.length; i += DELETE_BATCH_SIZE) {
|
||||
const batch = deletableSlugs.slice(i, i + DELETE_BATCH_SIZE);
|
||||
try {
|
||||
const deleted = await engine.deletePages(batch, deleteScopedOpts);
|
||||
reconciledDeletes += deleted.length;
|
||||
@@ -3290,6 +3332,34 @@ export function planReconcileDeletes(
|
||||
return { staleSlugs, reconcilableCount: reconcilable.length, massDelete };
|
||||
}
|
||||
|
||||
/**
|
||||
* #2426: every repo-relative path that ever appeared as an ADD in git history
|
||||
* (rename detection off, so a `git mv` destination still counts as an add).
|
||||
* Used by the full-sync reconcile to distinguish "file was committed and later
|
||||
* deleted" (genuine delete → reconcile) from "file was NEVER committed"
|
||||
* (DB-only write-through → preserve). Returns null when `repoPath` isn't a git
|
||||
* work tree or git is unavailable — callers keep the plain-directory behavior.
|
||||
* Forward-slash-normalized to match `normalizeReconcilePath` membership tests.
|
||||
*/
|
||||
export function listEverCommittedPaths(repoPath: string): Set<string> | null {
|
||||
let stdout: string;
|
||||
try {
|
||||
stdout = execFileSync(
|
||||
'git',
|
||||
['-C', repoPath, '-c', 'core.quotepath=off', 'log', '--all', '--no-renames',
|
||||
'--diff-filter=A', '--format=', '--name-only'],
|
||||
{ encoding: 'utf8', maxBuffer: 512 * 1024 * 1024, stdio: ['ignore', 'pipe', 'ignore'] },
|
||||
);
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
const set = new Set<string>();
|
||||
for (const line of stdout.split('\n')) {
|
||||
if (line) set.add(line.replace(/\\/g, '/'));
|
||||
}
|
||||
return set;
|
||||
}
|
||||
|
||||
/**
|
||||
* #2828 escape hatch: `GBRAIN_ALLOW_MASS_RECONCILE=1` restores the pre-valve
|
||||
* behavior for the rare intentional bulk removal. Env-only (an incident-time
|
||||
|
||||
@@ -183,15 +183,17 @@ if [ "\${1:-}" = "--push-only" ]; then
|
||||
fi
|
||||
|
||||
_msg="\${1:?usage: brain-commit-push.sh <message> <path> [paths...]}"; shift || true
|
||||
# Pull first so the local tree is current before we stage.
|
||||
git fetch origin >/dev/null 2>&1 || true
|
||||
git pull --rebase origin "$_branch" || { git rebase --abort >/dev/null 2>&1 || true; echo "rebase conflict: manual attention needed" >&2; exit 3; }
|
||||
|
||||
# EXPLICIT paths only — never a blind 'git add -A' (would risk committing
|
||||
# secrets, temp files, or unrelated edits).
|
||||
if [ "$#" -eq 0 ]; then
|
||||
echo "refusing blind 'git add -A' — pass explicit path(s) to commit" >&2; exit 2
|
||||
fi
|
||||
# COMMIT BEFORE PULL (#2426): the old order (fetch + pull --rebase, THEN stage)
|
||||
# aborted on any dirty tree — 'cannot pull with rebase: You have unstaged
|
||||
# changes' — so the helper could never commit a MODIFIED page (exactly the
|
||||
# write-through case). Stage + commit first; brain_push below already handles
|
||||
# a remote that advanced (push -> rejected -> pull --rebase -> push).
|
||||
git add -- "$@"
|
||||
if git diff --cached --quiet; then echo "nothing to commit"; exit 0; fi
|
||||
git commit -m "$_msg"
|
||||
@@ -337,6 +339,47 @@ function uninstallLocalHook(repoPath: string): boolean {
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* True when the gbrain durability post-commit hook is installed — i.e. the
|
||||
* user opted this repo into push-durability via `gbrain sources harden`.
|
||||
* Cheap (one git-config read + one file read); used as the gate for
|
||||
* write-through auto-commit (#2426).
|
||||
*/
|
||||
export function isDurabilityHardened(repoPath: string): boolean {
|
||||
try {
|
||||
const { dir } = resolveHooksDir(repoPath);
|
||||
const hookPath = join(dir, 'post-commit');
|
||||
return existsSync(hookPath) && readFileSync(hookPath, 'utf-8').includes(HOOK_BANNER);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* #2426: best-effort commit of a single write-through artifact so DB writes
|
||||
* reach git (the post-commit hook then background-pushes). Pre-fix,
|
||||
* write-through `.md` accumulated uncommitted forever: it never reached the
|
||||
* remote, froze `last_sync_at` (HEAD never moved), and a later `sync --full`
|
||||
* delete-reconcile treated the never-committed pages as disposable.
|
||||
*
|
||||
* Path-limited (`git commit -- <path>`) so unrelated staged/dirty edits are
|
||||
* never swept into the commit. Never throws; returns false on any failure
|
||||
* (index.lock contention, nothing changed, detached states) — the DB row and
|
||||
* the on-disk file remain the durable sinks either way.
|
||||
*/
|
||||
export function commitWriteThroughFile(repoPath: string, absPath: string, slug: string): boolean {
|
||||
try {
|
||||
const rel = relative(repoPath, absPath);
|
||||
if (!rel || rel.startsWith('..') || isAbsolute(rel)) return false;
|
||||
const gitOpts = { stdio: 'ignore', timeout: 30_000, env: { ...process.env, ...GIT_ENV } } as const;
|
||||
execFileSync('git', ['-C', repoPath, 'add', '--', rel], gitOpts);
|
||||
execFileSync('git', ['-C', repoPath, 'commit', '-m', `gbrain: write-through ${slug}`, '--', rel], gitOpts);
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
// ── Committed helper ────────────────────────────────────────────────────────
|
||||
|
||||
function installHelper(repoPath: string, dryRun: boolean): { status: StepStatus; detail: string } {
|
||||
|
||||
+8
-3
@@ -255,7 +255,12 @@ const PRUNE_DIR_NAMES = new Set<string>([
|
||||
// with the first-sync walker in commands/import.ts.
|
||||
'venv',
|
||||
'.raw',
|
||||
'ops',
|
||||
// NOTE (#2404): `'ops'` used to be in this list (a v0.2.0-era carve-out for
|
||||
// one brain layout). Matching the bare segment pruned EVERY user `ops/`
|
||||
// directory at any depth — sync silently deleted `ops/*` pages and never
|
||||
// imported `ops/*` files, while the bundled daily-task-manager skill
|
||||
// prescribes `ops/tasks` as its canonical page. `ops/` is ordinary content;
|
||||
// do NOT re-add it. Only generated/vendored trees belong here.
|
||||
]);
|
||||
|
||||
/**
|
||||
@@ -352,8 +357,8 @@ function classifySync(path: string, opts: SyncableOptions = {}): SyncableReason
|
||||
if (!isAllowedByStrategy(path, strategy)) return 'strategy';
|
||||
|
||||
// Skip every path segment that pruneDir would block walkers from descending
|
||||
// into. Catches hidden dirs (`.git`, `.obsidian`), `.raw/` sidecars,
|
||||
// `node_modules/` (latent bug fix), and `ops/` at any depth.
|
||||
// into. Catches hidden dirs (`.git`, `.obsidian`), `.raw/` sidecars, and
|
||||
// vendor/generated trees (`node_modules/`, `vendor/`, …) at any depth.
|
||||
const segments = path.split('/');
|
||||
if (segments.some(p => !pruneDir(p))) return 'pruned-dir';
|
||||
|
||||
|
||||
@@ -27,6 +27,7 @@ import { randomBytes } from 'crypto';
|
||||
import type { BrainEngine } from './engine.ts';
|
||||
import { serializePageToMarkdown, resolvePageFilePath } from './markdown.ts';
|
||||
import { isWriteTargetContained } from './path-confine.ts';
|
||||
import { isDurabilityHardened, commitWriteThroughFile } from './brain-repo-durability.ts';
|
||||
|
||||
/** Minimal logger surface — structurally compatible with operations.ts `Logger`. */
|
||||
export interface WriteThroughLogger {
|
||||
@@ -36,6 +37,13 @@ export interface WriteThroughLogger {
|
||||
export interface WriteThroughResult {
|
||||
written: boolean;
|
||||
path?: string;
|
||||
/**
|
||||
* True when the write was also committed to git (#2426). Only attempted on
|
||||
* repos hardened via `gbrain sources harden` (durability hook installed);
|
||||
* the hook then background-pushes the commit. Best-effort — a false/absent
|
||||
* value never blocks the write.
|
||||
*/
|
||||
committed?: boolean;
|
||||
/**
|
||||
* Non-error reasons the file was not written:
|
||||
* - no_repo_configured: the resolved target (source `local_path` or, for a
|
||||
@@ -157,7 +165,20 @@ export async function writePageThrough(
|
||||
throw writeErr;
|
||||
}
|
||||
|
||||
return { written: true, path: filePath };
|
||||
// #2426: on a durability-hardened repo (user ran `gbrain sources harden`),
|
||||
// commit the artifact so it reaches git — pre-fix, write-through content
|
||||
// stayed uncommitted forever: never pushed, `last_sync_at` frozen, and
|
||||
// silently deleted by a later `sync --full` delete-reconcile. The local
|
||||
// post-commit hook background-pushes the commit. Best-effort: a commit
|
||||
// failure never fails the write (the DB row + file are the durable sinks).
|
||||
let committed = false;
|
||||
try {
|
||||
if (isDurabilityHardened(writeRoot)) {
|
||||
committed = commitWriteThroughFile(writeRoot, filePath, slug);
|
||||
}
|
||||
} catch { /* best-effort */ }
|
||||
|
||||
return { written: true, path: filePath, ...(committed ? { committed } : {}) };
|
||||
} catch (e) {
|
||||
const msg = e instanceof Error ? e.message : String(e);
|
||||
opts.logger?.warn(`[write-through] failed for ${slug}: ${msg}`);
|
||||
|
||||
Reference in New Issue
Block a user