From bcf3b73dcf80a45aec3162cb565c01fd18661a3e Mon Sep 17 00:00:00 2001 From: Masa <98894508+Masashi-Ono0611@users.noreply.github.com> Date: Tue, 21 Jul 2026 06:06:21 +0900 Subject: [PATCH] fix(sync): self-heal a never-git-initialized default brain dir (#2964) (#2967) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(sync): self-heal a never-git-initialized default brain dir (#2964) The dream cycle's sync phase throws unconditionally on a legacy sync.repo_path-anchored default brain dir that was never git init-ed (predates git-backed sync, or was rsync'd without its .git), failing every nightly run with no recovery. doctor's sync_freshness/ sync_consolidation checks report "ok" for this exact brain, but only because they query the sources table (0 rows for a legacy default brain) — a coincidental false-negative, not a real diagnosis. Self-heal by git-initializing the dir and capturing the current on-disk state as the sync baseline, scoped to !opts.sourceId only — gbrain owns this directory outright, unlike a registered local source (sources add --path, no --url) which is the user's own external directory and should keep failing loudly. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01NCVEvUVm15bFuKqskWgfNS * fix(sync): dry-run no-write contract, unborn-HEAD recovery, no-gpg-sign (#2964) Codex review on the initial self-heal patch (b1671ee) found 3 real gaps: - P1: the self-heal ran even under --dry-run, mutating the filesystem during what's documented as a preview-only command. Gated the whole self-heal (both discoverGitRoot and the headCommit read) on !opts.dryRun, same as the existing !opts.sourceId ownership check. - P2: if `git init` succeeded but the process died before the baseline commit landed, the next run's discoverGitRoot would succeed (`.git` exists) and skip recovery entirely, permanently wedging on "No commits in repo" forever. Added the same self-heal at the `git rev-parse HEAD` catch site, sharing a new createSyncBaselineCommit helper with the discoverGitRoot catch. - P2: the baseline commit inherited the operator's global commit.gpgSign, which can block headless cron/launchd runs on an unavailable signing agent/pinentry. Added --no-gpg-sign. Two new tests cover dry-run no-mutation and unborn-HEAD recovery. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01NCVEvUVm15bFuKqskWgfNS * fix(sync): restrict git auto-init self-heal to the anchor-resolved path only (#2964) Second Codex review round (a10eeab) found the ownership check still too loose: - P1 (security): !opts.sourceId alone isn't proof gbrain owns repoPath. jobs.ts's `sync` job handler leaves sourceId undefined whenever job.data.repoPath doesn't match a registered source's local_path, so an admin-scope submit_job({name:'sync', data:{repoPath}}) MCP call could point the self-heal at an arbitrary directory and have it silently git-init + commit + ingest it. Gated both self-heal sites on !opts.repoPath too — only the path resolved from gbrain's own sync.repo_path anchor (never a caller-supplied one) is eligible. - P2: the unborn-HEAD recovery site calls discoverGitRoot, which walks UP from repoPath and can resolve to an ANCESTOR repo for a --src-subpath/subdir-as-repoPath sync with an unborn HEAD. Committing there would `git add -A` sibling files well outside the sync scope. Added a check that gitContextRoot === realpathSync(repoPath) before self-healing; refuses (falls through to the original error) otherwise. Tests rewritten to exercise the true self-heal-eligible path (anchor config via engine.setConfig('sync.repo_path', dir), no repoPath/sourceId passed) instead of an explicit repoPath, plus a new test asserting a caller-supplied repoPath with no sourceId still throws. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01NCVEvUVm15bFuKqskWgfNS * fix(sync): prove self-heal ownership by anchor VALUE, not field presence (#2964) Third Codex review round (613ad5b) found the previous round's fix broke the very call site it was meant to repair, plus a second scope gap: - P1 (critical): !opts.repoPath rejected self-heal on the REAL production callers too. runPhaseSync (dream cycle's sync phase, cycle.ts) always passes `repoPath: brainDir` explicitly after resolving it upstream, and the CLI's bare `gbrain sync` resolves sourceId='default'. Both made the ownership gate rethrow, leaving `gbrain dream` and `gbrain sync` wedged on the exact non-git legacy brain this fix targets — only synthetic callers that omitted both fields ever healed. Fixed by proving ownership by VALUE instead of by field absence: a new isAnchorOwnedSyncPath() re-reads gbrain's own persisted sync.repo_path config and requires the resolved repoPath to equal it exactly, regardless of whether the caller passed it explicitly or let it default. An attacker-supplied arbitrary path (e.g. via submit_job({name:'sync', data:{repoPath}})) only self-heals if it happens to already equal gbrain's own anchor — which is the legitimate case, not an escalation. opts.sourceId and opts.srcSubpath still disqualify unconditionally (registered/subpath-scoped syncs are a different ownership context). - P2: a --src-subpath sync with an unborn parent-repo HEAD would commit the whole ancestor root, capturing sibling files outside the scope. isAnchorOwnedSyncPath's opts.srcSubpath check closes this; the existing gitContextRoot === repoPath check stays as defense in depth. - P2: manageGitignore's "warn and return" contract (a deliberate side- effect that must never kill the sync job for its OTHER callers) meant a broken gbrain.yml or unwritable .gitignore would silently let the baseline `git add -A` commit db_only content. createSyncBaselineCommit now recomputes db_only exclusion directly from loadStorageConfig and passes it to `git add` as pathspecs, independent of the .gitignore write's success — true fail-closed. (A redundant pathspec exclude for a path .gitignore ALREADY covers makes git's -A bail with "paths ignored, use -f" even though the negation is correct, so each dir is check-ignore'd first and only pathspec-excluded when NOT already covered.) Tests rewritten around the anchor-VALUE model: the critical regression case (explicit repoPath matching the anchor still heals — the exact scenario Codex proved was broken) plus a true negative (a caller-supplied path that does NOT match the anchor still throws), --src-subpath refusal, and db_only fail-closed exclusion. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01NCVEvUVm15bFuKqskWgfNS * fix(sync): allow default-source ownership, realpath compare, generous timeout, --no-verify (#2964) Fourth Codex review round (9c1d461) found the previous round's ownership gate still didn't match the REAL installed-brain shape, plus 3 more gaps: - P1 (critical): rejecting all non-empty opts.sourceId meant self-heal still never fired on a real brain. Migration sources_table_additive seeds a 'default' source row whose local_path mirrors sync.repo_path on every brain that's run it (virtually all of them), so resolveSourceForDir (dream cycle) and bare `gbrain sync` both resolve sourceId:'default' in practice, never undefined. isAnchorOwnedSyncPath now permits sourceId undefined OR exactly 'default' (gbrain's own bootstrap identity, never something a caller names) and proves ownership by rereading the LIVE anchor for that same identity (sources.default.local_path vs config.sync.repo_path). - P2: compared raw anchor/repoPath strings, so a cosmetic difference (trailing slash, ..) between the stored anchor and dream.ts's path.resolve()-normalized brainDir would defeat the match. Now realpath-compares both sides (fail-closed on ENOENT/dangling). - P2: the shared git() helper's 30s timeout could abort the baseline `git add -A` on a large legacy brain mid-way, after `git init` already created `.git` — leaving an unborn repo every subsequent sync would retry and time out identically forever. Added an optional timeoutMs param (default unchanged at 30s); the baseline add call uses 10min. - P2: the baseline commit could trigger an operator's global core.hooksPath/init.templateDir hooks (pre-commit/commit-msg), breaking headless recovery if those hooks need project tooling or prompt. Added --no-verify. Tests: rewrote the mis-scoped "registered source" test (it used sourceId:'default', which is now correctly permitted) into two — a new regression test proving sourceId='default' + matching local_path heals (the actual production shape), and a corrected non-default-sourceId refusal test. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01NCVEvUVm15bFuKqskWgfNS * fix(sync): defer .gitignore write past first import, rebuild index, fail closed on unparseable db_only (#2964) Fifth Codex review round (c17cd23) found the baseline-commit helper interacting badly with the pre-existing db_only storage-tiering feature: - P1 (data loss): createSyncBaselineCommit called manageGitignore BEFORE performFullSync's collectSyncableFiles ran. collectSyncableFiles enumerates via `git ls-files --cached --others --exclude-standard`, so writing db_only entries into .gitignore first would silently exclude those pages from the DATABASE, not just from git — on a brain's very first sync. This is the exact bug class runSync's existing "manage .gitignore ONLY on successful sync" ordering (this file, ~line 4540, itself a prior Codex P1 fix, comment literally says so) was written to prevent — my new code reintroduced it in a different spot. Fix: stopped calling manageGitignore inside the self-heal at all. db_only exclusion for the COMMIT still happens via the existing pathspec computation (independent of .gitignore); .gitignore itself gets written by the already-existing post-sync flow once this sync completes, same as any other sync. - P1 (data leak): the unborn-HEAD recovery site can reach createSyncBaselineCommit with a repo whose INDEX already has entries staged from some prior operation (manual `git add`, interrupted workflow) before gbrain's self-heal ever touched it. `git add -A` only adds/updates — it doesn't drop an already-staged path our exclusion pathspecs now want excluded. Added `git read-tree --empty` to reset the index before staging (no-op on a freshly-`git init`-ed repo, whose index is already empty). - P2: loadStorageConfig warns-and-returns an EMPTY config (not a throw) for syntactically-valid-but-unsupported YAML (e.g. flow-style `db_only: [dir/]` — the narrow custom parser only handles block-style lists), which would silently resolve zero exclusions from a gbrain.yml that clearly intended some. Added a sniff-test: if gbrain.yml exists and mentions db_only but nothing resolved from it, refuse the baseline commit rather than guess "genuinely empty" vs "syntax silently ignored" (git init may already have run by this point — same "unborn, retry on next sync" recovery path handles it, and will hit this same guard again until the user fixes gbrain.yml). Tests: a positive regression proving db_only markdown IS imported into the DB on first sync (the actual data-loss scenario), the sniff-test refusal, and the stale-staged-content-gets-dropped case for the index rebuild. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01NCVEvUVm15bFuKqskWgfNS * fix(sync): post-heal .gitignore write, supabase_only alias, honor abort signal (#2964) Sixth Codex review round (e687913), 3 P2s: - Dream-cycle callers (cycle.ts:runPhaseSync) invoke performSync directly and never run runSync's CLI-only post-success manageGitignoreAtGitRoot. A brain self-healed only via the dream cycle would have db_only content correctly excluded from the baseline commit (createSyncBaselineCommit's pathspec exclusion) but no .gitignore ever written, leaving the user's own future manual git add/commit unprotected. Added performFullSyncAndMaybeGitignore, a thin wrapper around the 3 post-self-heal performFullSync call sites that writes .gitignore (same success-status gate runSync already uses) only when didSelfHeal is true — a no-op for the normal path, which still relies on runSync exactly as before. - The fail-closed sniff-test only checked the canonical `db_only` key; the deprecated-but-still-supported `supabase_only` alias (same keep-out-of-git semantics) could silently bypass it. Now checks both. - Self-heal didn't check opts.signal?.aborted before starting the (now up to 10-minute) git init + baseline commit, so a cancelled sync could still mutate disk and overrun its budget instead of returning partial. Added the check at both self-heal sites, before any git operation runs. New test proves .gitignore gets written after a bare performSync call (no runSync wrapper) — the actual dream-cycle shape. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01NCVEvUVm15bFuKqskWgfNS * fix(sync): neutralize a leftover .gitignore during the self-heal first sync (#2964) Seventh Codex review round (27337b0) ran an actual repro and caught the primary motivating scenario still broken: a brain rsync'd from another machine without its .git can retain that machine's old auto-managed .gitignore. collectSyncableFiles (inside performFullSync) enumerates via `git ls-files --exclude-standard`, so a leftover db_only ignore rule would silently omit those pages from THIS first sync's DATABASE import — the same bug class the round-6 ordering fix prevented for a .gitignore gbrain would have written itself, just triggered by a pre-existing file this time. Fix: performFullSyncAndMaybeGitignore now neutralizes any existing .gitignore for the duration of the one first-sync call — read, delete, restore byte-for-byte immediately after (even on error) — before manageGitignore re-merges the managed db_only block onto the restored original content. This matches exactly what a truly fresh brain with no .gitignore at all already does on its first sync (nothing to suppress collection there either); db_only content stays out of the git COMMIT independently via createSyncBaselineCommit's pathspec exclusion, which never depended on .gitignore. Test proves both halves: db_only markdown IS imported despite a leftover ignore rule, AND the user's own unrelated .gitignore lines (e.g. .DS_Store) survive the restore intact. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01NCVEvUVm15bFuKqskWgfNS * fix(sync): simplify — drop db_only-import machinery, isolate hooks fully (#2964) Eighth Codex review round (54c1e6f) found MORE problems with round 7's .gitignore-neutralization fix (deleting the whole file loses the user's own unrelated ignore rules; a multi-sync retry scenario could silently skip a still-broken db_only file while advancing the bookmark) plus 2 more issues in existing code. Rather than patch those too, stepped back and checked the actual documented semantics of db_only (docs/storage-tiering.md): it's for "bulk machine-generated content... written to disk as a local cache" — DB is the source of truth, disk is a cache populated FROM the DB (`export --restore-only` restores it), never the other way. Nothing in the docs says `gbrain sync`'s git-diff-based file collection is how db_only content is supposed to reach the database — that's ingest-specific tooling's job. Confirmed directly: `loadStorageConfig` returns the byte-identical `{db_tracked:[], db_only:[]}` for a malformed flow-style array AND a literal empty `db_only: []`, so rounds 6-7's "ensure db_only markdown gets imported on this first sync" chase was solving a problem outside sync's actual scope in the first place, on an increasingly complex, adversarially-discovered- edge-case foundation. Reverted: performFullSyncAndMaybeGitignore (the wrapper + didSelfHeal tracking + .gitignore neutralize/restore dance + post-success manageGitignore call). After self-heal, import and any subsequent .gitignore management now behave EXACTLY like any other brain, self-healed or not — runSync's existing post-success manageGitignoreAtGitRoot covers the CLI path identically either way; the dream cycle not calling it is a separate, pre-existing characteristic of the dream cycle in general (applies equally to an already-git-initialized brain going through the same path), not something this fix introduces. Kept (still correct, self-contained, don't depend on the reverted machinery): createSyncBaselineCommit's pathspec-based db_only exclusion for the COMMIT itself (matches the documented "not committed to git" requirement), the fail-closed sniff-test guard (now documents its known, structurally-unavoidable false-positive on a genuinely-empty `db_only: []` — the trade-off is deliberate: low-cost, self-resolving false positive vs. high-cost, hard-to-undo false negative), the index rebuild, and the 600s add timeout. Improved (round 8, P2): hooks isolation. --no-verify only skips pre-commit/commit-msg; added `-c core.hooksPath=/dev/null` for the baseline commit, which disables prepare-commit-msg and post-commit too (the latter runs synchronously inside the same git invocation and could otherwise hang past the timeout without even being the slow step). Tests: removed the 3 that exercised the reverted db_only-import machinery; the remaining 12 (ownership, dry-run, index rebuild, sniff test, commit-exclusion, unborn-HEAD recovery) are unaffected by the simplification. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01NCVEvUVm15bFuKqskWgfNS * fix(sync): unconditional db_only exclusion, literal pathspecs, precise sniff test (#2964) Ninth Codex review round (a6e07f6): - P1: the check-ignore pre-filter (skip pathspec-excluding a dir already covered by .gitignore) could be defeated by a pre-existing .gitignore that ignores a db_only tree with a wildcard but re-includes a child via negation (e.g. `private-cache/*` + `!private-cache/index.md`) — check-ignore on the directory still reports "ignored", so the filter skipped the pathspec exclusion, and `git add -A` staged the re-included child anyway. Fixed by making exclusion unconditional: every db_only dir is always pathspec-excluded now, never pre-filtered against .gitignore state at all — our own pathspec doesn't consult .gitignore, so no .gitignore content (negated or not) can defeat it. The advisory "paths ignored... use -f" error this can now trigger when a dir IS also already .gitignore'd (verified: git still stages everything else correctly despite the nonzero exit) is caught and swallowed by matching its exact stderr text; anything else rethrows. - P2: `:!dir` pathspec shorthand reinterprets a dir name that itself starts with a pathspec magic character (e.g. `:private/`) instead of excluding it literally. Switched to `:(exclude,literal)dir`. - P2: the fail-closed sniff-test's bare substring search on gbrain.yml's raw content could trip on a comment or unrelated prose mentioning "db_only" even when there's no real storage section at all, refusing self-heal forever on an unrelated false positive. Now requires an actual YAML key line (`db_only:`/`supabase_only:`, trimmed, ignoring `#` comments) — the round-8-documented "genuinely empty db_only: []" false positive is unchanged and remains an accepted trade-off (still structurally indistinguishable from unsupported syntax at the loadStorageConfig API boundary), but comment/prose mentions no longer false-positive. Not fixed (deliberately, documented trade-off — see PR description): Codex's other P1 this round (refuse baselining when other git refs/ history exist alongside an unborn HEAD) is a narrow, non-destructive scenario — self-heal only ever acts on the current branch ref when it's provably commit-less, never touches or deletes any other ref (remote- tracking, other branches), so at worst it creates a possibly-unexpected extra commit on an otherwise-empty branch the user hadn't checked out yet. Chasing it further trades diminishing real-world risk reduction against unbounded scope growth in what's fundamentally still the self-heal fix from round 1. Two new tests: unconditional exclusion despite a matching pre-existing .gitignore (proves the advisory-swallow path), and a comment-only gbrain.yml no longer false-positives the sniff test. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01NCVEvUVm15bFuKqskWgfNS --------- Co-authored-by: Claude Sonnet 5 --- src/commands/sync.ts | 303 ++++++++++++++++++++++++++++++- test/sync-git-autoinit.test.ts | 322 +++++++++++++++++++++++++++++++++ 2 files changed, 621 insertions(+), 4 deletions(-) create mode 100644 test/sync-git-autoinit.test.ts diff --git a/src/commands/sync.ts b/src/commands/sync.ts index 0e857003b..55177421b 100644 --- a/src/commands/sync.ts +++ b/src/commands/sync.ts @@ -919,10 +919,10 @@ export function buildAutoEmbedArgs(slugs: string[], sourceId?: string): string[] * 100 MiB is generous but still bounded — a 100K-file diff with long * paths tops out around 10–20 MiB in practice. */ -function git(repoPath: string, args: string[], configs: string[] = []): string { +function git(repoPath: string, args: string[], configs: string[] = [], timeoutMs = 30000): string { return execFileSync('git', buildGitInvocation(repoPath, args, configs), { encoding: 'utf-8', - timeout: 30000, + timeout: timeoutMs, maxBuffer: 100 * 1024 * 1024, }).trim(); } @@ -943,6 +943,171 @@ export function discoverGitRoot(inputPath: string): string { } } +/** + * #2964: snapshot the CURRENT on-disk state of a gbrain-owned brain dir as + * a baseline commit — used both right after a self-healing `git init` (no + * `.git` at all) and to recover a repo left with `.git` but zero commits + * (an interrupted prior self-heal, or a `git init` from some other source + * that never got a first commit). Respects `.gitignore` (written first) so + * future incremental syncs diff against what's actually here rather than + * an empty tree — an empty initial commit would make every existing file + * look "added" again on the next sync, even though the full-sync pass that + * follows already imported them from disk directly. + * + * `--no-gpg-sign` + explicit `-c user.name/user.email`: this runs from a + * headless nightly cron/launchd invocation, which has no reason to have + * git signing/identity configured, and must not block on an unavailable + * signing agent or pinentry prompt. + * + * db_only exclusion is recomputed directly and passed to `git add` as + * negative pathspecs, rather than relying solely on `manageGitignore` + * having written `.gitignore` successfully: that helper is deliberately + * best-effort (a broken gbrain.yml parse, or an unwritable .gitignore, + * only warns and returns — the right default for its OTHER callers, where + * .gitignore management is a side effect that must never kill the sync + * job). For a commit we are about to create ourselves, "fail open" there + * would mean silently committing db_only content into git history. Fail + * closed instead: db_only exclusion doesn't depend on the .gitignore + * write having succeeded. `loadStorageConfig` throwing (unreadable + * gbrain.yml, or a semantic overlap) propagates — better to leave this + * self-heal wedged with a clear error than commit unknown content. + */ +function createSyncBaselineCommit(repoPath: string): void { + // #2964: db_only exclusion is computed directly from loadStorageConfig + // and passed to `git add` as pathspecs — deliberately NOT via + // manageGitignore/.gitignore, for two independent reasons: + // + // 1. Ordering (Codex review round 6, P1): `collectSyncableFiles` — the + // file enumeration `performFullSync` runs right after this function + // returns — honors `.gitignore` via `git ls-files --exclude-standard`. + // Writing db_only entries into `.gitignore` BEFORE that first import + // would silently exclude those pages from the database entirely. + // That's the exact bug class `runSync`'s existing "manage .gitignore + // ONLY on successful sync" ordering (this file, `manageGitignoreAtGitRoot` + // callers below — itself a prior Codex P1 fix) exists to prevent. Leave + // `.gitignore` untouched here; the existing post-sync flow writes it + // once this sync completes, same as it does for every other sync. + // 2. Fail-closed (rounds 5-6): `manageGitignore`'s "warn and return" on a + // broken gbrain.yml/unwritable .gitignore is the right default for its + // OTHER callers (a side effect that must never kill the sync job), but + // wrong for a commit we are creating ourselves — silently committing + // db_only content into git history. + const storageConfig = loadStorageConfig(repoPath); + const dbOnlyDirs = storageConfig?.db_only ?? []; + // Sniff-test fail-closed (round 6, P2): `loadStorageConfig` warns-and- + // returns an EMPTY config for syntactically-valid-but-unsupported YAML + // (e.g. flow-style `db_only: [dir/]` — the narrow custom parser only + // handles block-style lists), which would silently resolve zero + // exclusions from a file that clearly intended some. If gbrain.yml + // exists and mentions db_only (or its deprecated pre-v0.22.11 alias + // `supabase_only` — same keep-out-of-git semantics, still a supported + // backward-compat key per storage-config.ts) but nothing resolved from + // it, refuse rather than guess "genuinely empty" vs "syntax ignored". + // + // Known false-positive (round 8 review): a genuinely, intentionally + // empty `db_only: []` mentioning the word also refuses, and can't be + // told apart from the unsupported-syntax case — `loadStorageConfig` + // returns the IDENTICAL `{db_tracked:[],db_only:[]}` for both (verified + // directly: flow-style `[dir/]` and literal `[]` both collapse to that + // same shape). Distinguishing them would mean teaching this function + // about the parser's internal line-recognition rules, which belongs in + // storage-config.ts, not here. Accepted trade-off: the false-positive + // cost is low and self-resolving (the brain stays wedged with a clear, + // actionable error until the user drops the pointless empty stanza or + // fixes their syntax; retried on every subsequent sync); the + // false-negative this guards against — silently committing db_only + // content into permanent git history — is high-cost and hard to undo. + if (dbOnlyDirs.length === 0) { + const yamlPath = join(repoPath, 'gbrain.yml'); + const yamlContent = existsSync(yamlPath) ? readFileSync(yamlPath, 'utf-8') : ''; + // A YAML KEY line (`db_only:` / `supabase_only:`, ignoring leading + // whitespace and `#` comments), not a bare substring search — round 9, + // P2: a comment or unrelated prose value that happens to mention the + // word (e.g. `# db_only handling TBD`) must not trip this guard on an + // otherwise-genuinely-config-free gbrain.yml. + const mentionsUnresolvedKey = yamlContent.split('\n').some((line) => { + const trimmed = line.trim(); + return !trimmed.startsWith('#') && /^(db_only|supabase_only)\s*:/.test(trimmed); + }); + if (mentionsUnresolvedKey) { + throw new Error( + `${yamlPath} mentions db_only but no directories resolved from it — refusing to ` + + `auto-commit (cannot tell "genuinely empty" from "unsupported syntax silently ignored"). ` + + `Fix gbrain.yml's storage.db_only syntax, or git-init this directory manually.`, + ); + } + } + // #2964 (round 9, P1): every db_only dir is ALWAYS pathspec-excluded, + // unconditionally — never pre-filtered against what an existing + // `.gitignore` claims to already cover. An earlier version checked + // `git check-ignore -q dir` first and skipped the pathspec when it + // already reported "ignored" (to dodge the advisory error below), but + // `check-ignore` on a directory can say "ignored" even when a + // pre-existing `.gitignore` re-includes a child via negation (e.g. + // `private-cache/*` + `!private-cache/index.md`) — the filter would + // then skip excluding it via pathspec, and `git add -A` would stage + // that re-included child despite the whole directory being declared + // db_only. Our OWN pathspec exclusion is unconditional and doesn't + // consult `.gitignore` at all, so it can't be defeated by ANY + // .gitignore content, negated or not. `:(exclude,literal)dir` (not the + // `:!dir` shorthand) so a db_only dir name that itself starts with a + // pathspec magic character like `:` is excluded literally rather than + // reinterpreted (round 9, P2). + const excludePathspecs = dbOnlyDirs.map((dir) => `:(exclude,literal)${dir}`); + // Clear the index before staging (round 6, P1): the unborn-HEAD + // recovery site can reach this function with a repo whose index + // already has entries staged from some OTHER prior operation (a manual + // `git add`, an interrupted workflow) before gbrain ever touched it. + // `add -A` only adds/updates — it does not drop an already-staged path + // that our exclusion pathspecs above now want excluded. `read-tree + // --empty` resets the index without touching the working tree; a + // no-op on a freshly-`git init`-ed repo, whose index is already empty. + git(repoPath, ['read-tree', '--empty']); + try { + // #2964: 10 minutes, not the shared git() helper's 30s default — this + // full-tree `git add -A` walks a legacy brain that may hold years of + // accumulated content. A 30s timeout would abort staging after `git + // init` already created `.git`, leaving an unborn repo that every + // subsequent sync would retry (and time out identically) forever; + // the unborn-HEAD recovery path exists for OTHER causes of that + // state, not to be this one's normal first outcome. + git(repoPath, ['add', '-A', '--', '.', ...excludePathspecs], [], 600_000); + } catch (err) { + // Now that exclusion is always applied (never pre-filtered), an + // explicit pathspec exclusion for a path a pre-existing `.gitignore` + // ALSO happens to cover trips git's advice.addIgnoredFile: nonzero + // exit + "paths ignored by one of your .gitignore files, use -f", + // even though the add otherwise fully succeeded (verified directly: + // `git status --short` right after this exact error shows every + // non-excluded path staged correctly). Recognize and swallow ONLY + // this exact advisory; anything else (timeout, permission denied, + // real corruption) rethrows. + const stderr = err && typeof err === 'object' && 'stderr' in err ? String((err as { stderr: unknown }).stderr) : ''; + if (!stderr.includes('ignored by one of your .gitignore files')) throw err; + } + git( + repoPath, + // --no-verify only skips pre-commit/commit-msg — prepare-commit-msg + // and (worse, since it runs AFTER the commit object already exists, + // synchronously inside this same git invocation) post-commit are + // NOT covered by it. An operator's global core.hooksPath or + // init.templateDir can wire either, expecting project tooling, + // prompting interactively, or hanging — none of which a headless + // self-heal commit can satisfy, and a hanging post-commit hook would + // burn the 600s budget above without even being the slow step. + // `-c core.hooksPath=/dev/null` (in configs, below) makes git look + // for hook scripts inside a location that can't contain any, + // disabling the entire hooks path for this one invocation — the + // complete form of what --no-verify only partially covers, kept for + // explicitness on the two hooks it does name. + [ + 'commit', '--quiet', '--allow-empty', '--no-gpg-sign', '--no-verify', + '-m', 'gbrain: initial commit (auto-init by sync)', + ], + ['user.name=gbrain', 'user.email=gbrain@localhost', 'core.hooksPath=/dev/null'], + ); +} + /** * #774 NAV-1 TOCTOU: true only if filePath realpath-resolves inside gitRoot. * Guards symlink escape at the per-file level (a committed symlink whose @@ -1009,6 +1174,65 @@ async function readSyncAnchor( return await engine.getConfig(`sync.${which}`); } +/** + * #2964: is `repoPath` gbrain's own default-brain anchor, as opposed to a + * path some caller merely happened to pass through unchanged? + * + * `!opts.sourceId` alone is NOT sufficient — and neither is rejecting + * `opts.sourceId` outright: migration `sources_table_additive` (v20) + * seeds a `'default'` source row whose `local_path` is copied FROM + * `config.sync.repo_path` on every brain that has ever run it (i.e. + * effectively all of them by now), and `writeSyncAnchor` keeps that row's + * `local_path` current on every sync thereafter. So on a real installed + * brain, `resolveSourceForDir` (dream cycle) and the CLI's bare `gbrain + * sync` both resolve `sourceId: 'default'`, NOT `undefined` — rejecting + * all non-empty `sourceId` (an earlier, insufficiently-reviewed version + * of this check) made self-heal never fire on that real path either, + * masked in tests only because a freshly-`initSchema()`'d test brain's + * `'default'` row has a null `local_path` (Codex review round 5). + * + * The actual boundary: `'default'` is gbrain's own bootstrap identity, + * not something a caller names — a DIFFERENT, non-default `sourceId` is + * what an explicit `sources add --path ` registration (a + * user's own external directory) looks like, and that's what must keep + * failing loudly. So: permit `sourceId` when it's exactly `undefined` or + * `'default'`, reject any other id, and for BOTH permitted cases prove + * ownership by VALUE — reread the live anchor for that same identity + * (`sources.default.local_path` when sourceId='default', else + * `config.sync.repo_path`) and require the resolved `repoPath` to + * REALPATH-equal it (not raw string equality: `dream`'s `resolveBrainDir` + * normalizes via `path.resolve`, so a trailing slash or `..` in the + * stored anchor must not defeat the match — Codex review round 5, P2). + * An arbitrary caller-supplied path (e.g. an admin-scope + * `submit_job({name:'sync', data:{repoPath}})`) only passes this check + * if it already equals gbrain's own anchor by realpath identity — at + * which point self-healing it is exactly the legitimate case, not an + * escalation. + * + * `opts.srcSubpath` disqualifies unconditionally: a subpath-scoped sync + * only wants THAT subdirectory captured, but the self-heal baseline + * commit runs `git add -A` at the git root (there's no file list yet to + * scope it to — collection happens after this point) — see the P2 review + * finding on `createSyncBaselineCommit`'s callers. + */ +async function isAnchorOwnedSyncPath( + engine: BrainEngine, + opts: SyncOpts, + repoPath: string, +): Promise { + if (opts.srcSubpath) return false; + if (opts.sourceId && opts.sourceId !== 'default') return false; + const anchor = await readSyncAnchor(engine, opts.sourceId, 'repo_path'); + if (anchor === null) return false; + try { + return realpathSync(anchor) === realpathSync(repoPath); + } catch { + // Anchor or repoPath doesn't realpath-resolve (dangling/nonexistent) — + // can't prove identity, so don't self-heal. + return false; + } +} + async function writeSyncAnchor( engine: BrainEngine, sourceId: string | undefined, @@ -1628,7 +1852,33 @@ async function performSyncInner(engine: BrainEngine, opts: SyncOpts): Promise --path + * ` registration of a user's own external directory looks like), + * AND the resolved `repoPath` must realpath-equal the LIVE anchor for + * that same identity. A caller-supplied path that does not match (a + * registered non-default source, or an admin-scope + * `submit_job({name:'sync', data:{repoPath}})` MCP call with an + * unrelated path) must keep failing loudly rather than being silently + * git-initialized without consent. + */ + +import { describe, test, expect, beforeAll, afterAll, beforeEach, afterEach } from 'bun:test'; +import { mkdtempSync, writeFileSync, rmSync, existsSync } from 'fs'; +import { join } from 'path'; +import { tmpdir } from 'os'; +import { PGLiteEngine } from '../src/core/pglite-engine.ts'; +import { resetPgliteState } from './helpers/reset-pglite.ts'; + +function mdPage(title: string, body = 'Content.'): string { + return `---\ntype: note\ntitle: ${title}\n---\n\n${body}`; +} + +describe('#2964: sync auto-inits a never-git-initialized default brain dir', () => { + let engine: PGLiteEngine; + let dir: string; + + beforeAll(async () => { + engine = new PGLiteEngine(); + await engine.connect({}); + await engine.initSchema(); + }, 60_000); + + afterAll(async () => { + await engine.disconnect(); + }, 60_000); + + beforeEach(async () => { + await resetPgliteState(engine); + dir = mkdtempSync(join(tmpdir(), 'gbrain-2964-')); + writeFileSync(join(dir, 'page1.md'), mdPage('Page 1')); + writeFileSync(join(dir, 'page2.md'), mdPage('Page 2')); + // The self-heal-eligible anchor: gbrain's own persisted config, not a + // caller-supplied --repo / job.data.repoPath (those are proven by + // VALUE against this anchor, not by mere absence — see file docstring). + await engine.setConfig('sync.repo_path', dir); + }); + + afterEach(() => { + if (dir) rmSync(dir, { recursive: true, force: true }); + }); + + test('anchor-resolved sync (no repoPath, no sourceId) on a non-git dir auto-inits git and imports files', async () => { + const { performSync } = await import('../src/commands/sync.ts'); + expect(existsSync(join(dir, '.git'))).toBe(false); + + const result = await performSync(engine, { noPull: true, noEmbed: true, full: true }); + + expect(result.status).toBe('first_sync'); + expect(result.added).toBe(2); + expect(existsSync(join(dir, '.git'))).toBe(true); + expect(await engine.getPage('page1')).not.toBeNull(); + expect(await engine.getPage('page2')).not.toBeNull(); + }); + + test('explicit repoPath matching the anchor still auto-inits (mirrors gbrain dream\'s sync phase)', async () => { + // cycle.ts's runPhaseSync (the actual dream-cycle call site this bug + // was filed against) always passes `repoPath: brainDir` explicitly — + // it already resolved the anchor itself upstream and threads it + // through. Gating self-heal on `!opts.repoPath` would silently never + // fire here; ownership must be proven by matching the anchor's VALUE. + const { performSync } = await import('../src/commands/sync.ts'); + expect(existsSync(join(dir, '.git'))).toBe(false); + + const result = await performSync(engine, { repoPath: dir, noPull: true, noEmbed: true, full: true }); + + expect(result.status).toBe('first_sync'); + expect(result.added).toBe(2); + expect(existsSync(join(dir, '.git'))).toBe(true); + }); + + test('a second sync after auto-init sees no changes (baseline commit captured current on-disk state)', async () => { + const { performSync } = await import('../src/commands/sync.ts'); + const first = await performSync(engine, { noPull: true, noEmbed: true, full: true }); + expect(first.added).toBe(2); + + // No new files, no explicit `full` — a real incremental sync against the + // auto-init baseline. Before this fix there was no baseline to diff + // against (sync errored outright); a naive fix that skipped the initial + // commit would make this call re-report both files as "added" again. + const second = await performSync(engine, { noPull: true, noEmbed: true }); + expect(second.status).not.toBe('first_sync'); + expect(second.added).toBe(0); + expect(second.modified).toBe(0); + }); + + test("sourceId='default' whose local_path mirrors the anchor still auto-inits (P1: the real installed-brain shape)", async () => { + // Migration sources_table_additive seeds a 'default' source row with + // local_path copied from sync.repo_path on every brain that's run it + // — i.e. this, not a bare no-sourceId call, is what runPhaseSync/CLI + // `gbrain sync` actually resolve to on a real installed brain. + await engine.executeRaw(`UPDATE sources SET local_path = $1 WHERE id = 'default'`, [dir]); + const { performSync } = await import('../src/commands/sync.ts'); + expect(existsSync(join(dir, '.git'))).toBe(false); + + const result = await performSync(engine, { + repoPath: dir, + sourceId: 'default', + noPull: true, + noEmbed: true, + full: true, + }); + + expect(result.status).toBe('first_sync'); + expect(result.added).toBe(2); + expect(existsSync(join(dir, '.git'))).toBe(true); + }); + + test('a registered non-default local source (sourceId != default, no remote_url) on a non-git dir still throws — not auto-inited', async () => { + await engine.executeRaw( + `INSERT INTO sources (id, name, local_path, config) VALUES ('mysource', 'mysource', $1, '{}'::jsonb)`, + [dir], + ); + const { performSync } = await import('../src/commands/sync.ts'); + await expect( + performSync(engine, { + repoPath: dir, + sourceId: 'mysource', + noPull: true, + noEmbed: true, + full: true, + }), + ).rejects.toThrow(/git repository/i); + expect(existsSync(join(dir, '.git'))).toBe(false); + }); + + test('a caller-supplied repoPath that does NOT match the anchor still throws (P1: MCP submit_job arbitrary-path guard)', async () => { + // Mirrors jobs.ts: submit_job({name:'sync', data:{repoPath}}) reaches + // performSyncInner with sourceId left undefined whenever repoPath + // doesn't match a registered source's local_path. Self-heal must not + // fire for a path that isn't gbrain's own anchor, even with no + // sourceId set — only exact anchor-value equality (the previous test) + // is eligible. + const other = mkdtempSync(join(tmpdir(), 'gbrain-2964-other-')); + writeFileSync(join(other, 'unrelated.md'), mdPage('Unrelated')); + try { + const { performSync } = await import('../src/commands/sync.ts'); + await expect( + performSync(engine, { repoPath: other, noPull: true, noEmbed: true, full: true }), + ).rejects.toThrow(/git repository/i); + expect(existsSync(join(other, '.git'))).toBe(false); + } finally { + rmSync(other, { recursive: true, force: true }); + } + }); + + test('--src-subpath on the anchor-resolved path still throws — not auto-inited (P2: subpath scope guard)', async () => { + // A self-heal baseline commit runs `git add -A` at the git root before + // any subpath-scoped file collection happens, so it would capture + // sibling directories a --src-subpath sync never intended to touch. + const { performSync } = await import('../src/commands/sync.ts'); + await expect( + performSync(engine, { + srcSubpath: 'wiki', + noPull: true, + noEmbed: true, + full: true, + }), + ).rejects.toThrow(/git repository/i); + expect(existsSync(join(dir, '.git'))).toBe(false); + }); + + test('--dry-run on the anchor-resolved path throws without writing anything to disk', async () => { + const { performSync } = await import('../src/commands/sync.ts'); + await expect( + performSync(engine, { repoPath: dir, dryRun: true, noPull: true, noEmbed: true, full: true }), + ).rejects.toThrow(/git repository/i); + // The whole point of --dry-run is "preview only" — it must never git-init + // or commit on our behalf, even though this is otherwise self-heal-eligible. + expect(existsSync(join(dir, '.git'))).toBe(false); + }); + + test('unborn-HEAD recovery: a bare `git init` with zero commits (interrupted prior self-heal) still completes', async () => { + const { performSync } = await import('../src/commands/sync.ts'); + const { execSync } = await import('child_process'); + // Simulate a self-heal that ran `git init` but died before the baseline + // commit landed (process killed, disk full, etc.) — `.git` exists so + // discoverGitRoot succeeds, but `git rev-parse HEAD` still fails. + execSync('git init -q', { cwd: dir }); + + const result = await performSync(engine, { repoPath: dir, noPull: true, noEmbed: true, full: true }); + + expect(result.status).toBe('first_sync'); + expect(result.added).toBe(2); + expect(execSync('git rev-parse HEAD', { cwd: dir }).toString().trim()).not.toBe(''); + }); + + test('db_only paths are excluded from the baseline commit even without gbrain.yml write support (P2: fail-closed exclusion)', async () => { + const { performSync } = await import('../src/commands/sync.ts'); + const { mkdirSync } = await import('fs'); + const { execSync } = await import('child_process'); + mkdirSync(join(dir, 'private-cache')); + writeFileSync(join(dir, 'private-cache', 'secret.bin'), 'binary-ish content'); + writeFileSync( + join(dir, 'gbrain.yml'), + 'storage:\n db_only:\n - private-cache\n', + ); + + await performSync(engine, { noPull: true, noEmbed: true, full: true }); + + expect(existsSync(join(dir, '.git'))).toBe(true); + const tracked = execSync('git ls-files', { cwd: dir }).toString(); + expect(tracked).not.toContain('private-cache'); + }); + + test('db_only exclusion applies even when a pre-existing .gitignore already covers the same dir (round 9 P1: unconditional pathspec)', async () => { + // Regression for the "check-ignore pre-filter" version of this logic: + // when a dir is ALSO already covered by an existing .gitignore, git's + // `-A` bails with an advisory "paths ignored... use -f" even though + // the add otherwise succeeds. Exclusion must be unconditional and the + // advisory must not surface as a hard failure. + const { performSync } = await import('../src/commands/sync.ts'); + const { mkdirSync } = await import('fs'); + const { execSync } = await import('child_process'); + mkdirSync(join(dir, 'private-cache')); + writeFileSync(join(dir, 'private-cache', 'secret.bin'), 'binary-ish content'); + writeFileSync(join(dir, 'gbrain.yml'), 'storage:\n db_only:\n - private-cache\n'); + writeFileSync(join(dir, '.gitignore'), 'private-cache/\n'); + + const result = await performSync(engine, { noPull: true, noEmbed: true, full: true }); + + expect(result.status).toBe('first_sync'); + const tracked = execSync('git ls-files', { cwd: dir }).toString(); + expect(tracked).not.toContain('private-cache'); + }); + + test('a comment merely mentioning db_only does not false-positive the sniff test (round 9 P2)', async () => { + const { performSync } = await import('../src/commands/sync.ts'); + // No `storage:` section at all — just a comment mentioning the word. + // A bare substring search would wrongly refuse this brain forever. + writeFileSync(join(dir, 'gbrain.yml'), '# db_only handling: TBD, not configured yet\n'); + + const result = await performSync(engine, { noPull: true, noEmbed: true, full: true }); + + expect(result.status).toBe('first_sync'); + expect(result.added).toBe(2); + }); + + test('a gbrain.yml that mentions db_only but resolves no dirs refuses the baseline commit (round 6 P2: unsupported-syntax sniff test)', async () => { + const { performSync } = await import('../src/commands/sync.ts'); + const { execSync } = await import('child_process'); + // Flow-style array — valid YAML, but the narrow custom parser only + // handles block-style lists, so loadStorageConfig warns and resolves + // an empty db_only list rather than throwing. + writeFileSync(join(dir, 'gbrain.yml'), 'storage:\n db_only: [private-cache/]\n'); + + await expect( + performSync(engine, { noPull: true, noEmbed: true, full: true }), + ).rejects.toThrow(/db_only/i); + // `git init` (site 1's first step) already ran before the sniff-test + // guard (inside createSyncBaselineCommit) refused — that's fine, it's + // the same "unborn repo" state the round-6-P1 index-rebuild test above + // recovers from on a later retry, which would hit this same guard and + // refuse again until gbrain.yml is fixed. What must NOT happen is a + // commit landing with unknown/unexcluded content. + expect(existsSync(join(dir, '.git'))).toBe(true); + let hasCommit = true; + try { + execSync('git rev-parse HEAD', { cwd: dir, stdio: 'pipe' }); + } catch { + hasCommit = false; + } + expect(hasCommit).toBe(false); + }); + + test('unborn-HEAD recovery drops stale staged content the exclusion pathspec now wants excluded (round 6 P1: index rebuild)', async () => { + const { performSync } = await import('../src/commands/sync.ts'); + const { mkdirSync } = await import('fs'); + const { execSync } = await import('child_process'); + mkdirSync(join(dir, 'private-cache')); + writeFileSync(join(dir, 'private-cache', 'secret.bin'), 'binary-ish content'); + writeFileSync(join(dir, 'gbrain.yml'), 'storage:\n db_only:\n - private-cache\n'); + // Simulate an interrupted workflow that left this file staged in an + // unborn repo BEFORE gbrain's self-heal ever ran. + execSync('git init -q', { cwd: dir }); + execSync('git add private-cache/secret.bin', { cwd: dir }); + + await performSync(engine, { noPull: true, noEmbed: true, full: true }); + + const tracked = execSync('git ls-files', { cwd: dir }).toString(); + expect(tracked).not.toContain('private-cache'); + }); + +});