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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NCVEvUVm15bFuKqskWgfNS

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();
 }
@@ -992,10 +992,24 @@ function createSyncBaselineCommit(repoPath: string, engineKind?: 'pglite' | 'pos
       }
     })
     .map((dir) => `:!${dir}`);
-  git(repoPath, ['add', '-A', '--', '.', ...excludePathspecs]);
+  // #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);
   git(
     repoPath,
-    ['commit', '--quiet', '--allow-empty', '--no-gpg-sign', '-m', 'gbrain: initial commit (auto-init by sync)'],
+    // --no-verify: skip pre-commit/commit-msg hooks. An operator's global
+    // core.hooksPath or init.templateDir can wire hooks that expect
+    // project tooling, prompt interactively, or reject the snapshot —
+    // none of which a headless self-heal commit can satisfy.
+    [
+      'commit', '--quiet', '--allow-empty', '--no-gpg-sign', '--no-verify',
+      '-m', 'gbrain: initial commit (auto-init by sync)',
+    ],
     ['user.name=gbrain', 'user.email=gbrain@localhost'],
   );
 }
@@ -1067,20 +1081,39 @@ async function readSyncAnchor(
 }
 
 /**
- * #2964: is `repoPath` gbrain's own legacy default anchor, as opposed to a
+ * #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: `runPhaseSync` (dream cycle)
+ * `!opts.sourceId` alone is NOT sufficient — and neither is rejecting
- * and `submit_job({name:'sync', data:{repoPath}})` (MCP, jobs.ts) both
+ * `opts.sourceId` outright: migration `sources_table_additive` (v20)
- * reach `performSyncInner` with `sourceId` undefined AND `opts.repoPath`
+ * seeds a `'default'` source row whose `local_path` is copied FROM
- * set explicitly — the former legitimately (it already resolved the
+ * `config.sync.repo_path` on every brain that has ever run it (i.e.
- * anchor itself and is passing it through), the latter potentially with
+ * effectively all of them by now), and `writeSyncAnchor` keeps that row's
- * an attacker-controlled path. The two are indistinguishable from
+ * `local_path` current on every sync thereafter. So on a real installed
- * `opts.repoPath`'s mere presence/absence, so ownership has to be proven
+ * brain, `resolveSourceForDir` (dream cycle) and the CLI's bare `gbrain
- * by VALUE: read the anchor fresh from config and require the resolved
+ * sync` both resolve `sourceId: 'default'`, NOT `undefined` — rejecting
- * `repoPath` to equal it exactly. An arbitrary caller-supplied path only
+ * all non-empty `sourceId` (an earlier, insufficiently-reviewed version
- * passes this check if it happens to already equal gbrain's own anchor —
+ * of this check) made self-heal never fire on that real path either,
- * at which point self-healing it is exactly the legitimate case.
+ * 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 <id> --path <dir>` 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
@@ -1093,9 +1126,17 @@ async function isAnchorOwnedSyncPath(
   opts: SyncOpts,
   repoPath: string,
 ): Promise<boolean> {
-  if (opts.sourceId || opts.srcSubpath) return false;
+  if (opts.srcSubpath) return false;
-  const anchor = await readSyncAnchor(engine, undefined, 'repo_path');
+  if (opts.sourceId && opts.sourceId !== 'default') return false;
-  return anchor !== null && anchor === repoPath;
+  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(

test/sync-git-autoinit.test.ts

@@ -1,29 +1,40 @@
 /**
  * #2964 — sync phase self-heals a never-git-initialized default brain dir.
  *
- * A legacy `sync.repo_path`-anchored default brain (no `sources` row) can
+ * A legacy `sync.repo_path`-anchored default brain can reach `performSync`
- * reach `performSync` pointed at a directory that was never `git init`-ed
+ * pointed at a directory that was never `git init`-ed (predates git-backed
- * (predates git-backed sync, or was rsync'd from another machine without
+ * sync, or was rsync'd from another machine without its `.git`). Before
- * its `.git`). Before this fix, `discoverGitRoot` threw unconditionally and
+ * this fix, `discoverGitRoot` threw unconditionally and the dream cycle's
- * the dream cycle's sync phase failed every night with no self-recovery,
+ * sync phase failed every night with no self-recovery, even though
- * even though `doctor`'s sync checks reported "ok" (for an unrelated
+ * `doctor`'s sync checks reported "ok" (for an unrelated reason — they
- * reason — they only look at the `sources` table, which has no rows for
+ * only look at the `sources` table in a way this brain shape doesn't hit).
- * this kind of brain).
  *
  * gbrain owns that directory outright, so the fix self-heals by `git
  * init`-ing it and capturing the current on-disk state as the sync
  * baseline. Ownership is proven by VALUE — the resolved `repoPath` must
- * equal gbrain's own persisted `sync.repo_path` anchor — not by whether
+ * realpath-equal gbrain's own anchor — not by whether
- * `opts.repoPath`/`opts.sourceId` happen to be set. That distinction
+ * `opts.repoPath`/`opts.sourceId` happen to be set:
- * matters because the real production caller (`runPhaseSync` in
+ *
- * cycle.ts, i.e. `gbrain dream`'s sync phase) always passes the resolved
+ * - Gating on `!opts.repoPath` (round 3) would have made self-heal never
- * anchor through explicitly as `opts.repoPath`; gating on
+ *   fire on `runPhaseSync` (dream cycle), which always resolves the
- * `!opts.repoPath` (an earlier, insufficiently-reviewed version of this
+ *   anchor itself and passes it through explicitly as `opts.repoPath`.
- * fix) would have made the self-heal never fire on the exact path it was
+ * - Gating on `!opts.sourceId` (round 4) would ALSO never fire in
- * written to repair (Codex review round 3 caught this). A caller-supplied
+ *   practice: migration `sources_table_additive` seeds a `'default'`
- * path that does NOT match the anchor (a registered local source, or an
+ *   source row whose `local_path` mirrors `sync.repo_path` on every
- * admin-scope `submit_job({name:'sync', data:{repoPath}})` MCP call with
+ *   brain that's run it (i.e. virtually all installed brains today), so
- * an unrelated path) must keep failing loudly rather than being silently
+ *   both the dream cycle and bare `gbrain sync` resolve
+ *   `sourceId: 'default'`, never `undefined`, in reality — a fresh test
+ *   brain's null `local_path` masked this (Codex review round 5).
+ *
+ * The actual boundary implemented by `isAnchorOwnedSyncPath`: `sourceId`
+ * must be `undefined` OR exactly `'default'` (gbrain's own bootstrap
+ * identity — a DIFFERENT id is what an explicit `sources add <id> --path
+ * <dir>` 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.
  */
 
@@ -111,12 +122,38 @@ describe('#2964: sync auto-inits a never-git-initialized default brain dir', ()
     expect(second.modified).toBe(0);
   });
 
-  test('a registered local source (sourceId set, no remote_url) on a non-git dir still throws — not auto-inited', async () => {
+  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: 'default',
+        sourceId: 'mysource',
         noPull: true,
         noEmbed: true,
         full: true,