fix(sync): keep the expected discover_git_root probe failure off stderr (#3232)

discoverGitRoot() probes `git rev-parse --show-toplevel` to locate the repo
root; a miss is expected/routine (a non-git-yet brain dir, a scratch dir) and
is either self-healed via auto git-init or surfaced as a friendlier Error.
Node's execFileSync writes the child's stderr straight to the parent's real
stderr by default unless an explicit `stdio` array is given, so every routine
probe miss dumped git's raw "fatal: not a git repository ..." line into
gbrain's operator logs -- indistinguishable from an actual crash to an
operator grepping logs for "fatal:" as a crash signature.

Add an opt-in `silenceStderr` param to the shared `git()` helper (sets
`stdio: ['ignore', 'pipe', 'pipe']`, which disables the implicit
passthrough-to-parent-stderr behavior) and pass it only from
discoverGitRoot's internal probe call. Every other `git()` call site is
unchanged, so unexpected-failure visibility elsewhere is preserved.

Related to #2964, which added the auto-recovery this probe feeds.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

src/commands/sync.ts

@@ -937,12 +937,28 @@ export function resolveNoEmbed(
  *
  * 100 MiB is generous but still bounded — a 100K-file diff with long
  * paths tops out around 10–20 MiB in practice.
+ *
+ * `silenceStderr`: Node's `execFileSync` writes the child's stderr straight
+ * through to the parent's real stderr by default (in addition to attaching
+ * it to the thrown error's `.stderr`) *unless* an explicit `stdio` array is
+ * given. Callers that treat a failure as an expected, self-handled outcome
+ * (rather than a crash to surface) pass `silenceStderr: true` so git's raw
+ * `fatal: ...` line never reaches the process's own stderr — only the
+ * caller's own (usually friendlier) handling of the caught error does.
+ * Default `false` preserves today's passthrough for every other call site.
  */
-function git(repoPath: string, args: string[], configs: string[] = [], timeoutMs = 30000): string {
+function git(
+  repoPath: string,
+  args: string[],
+  configs: string[] = [],
+  timeoutMs = 30000,
+  { silenceStderr = false }: { silenceStderr?: boolean } = {},
+): string {
   return execFileSync('git', buildGitInvocation(repoPath, args, configs), {
     encoding: 'utf-8',
     timeout: timeoutMs,
     maxBuffer: 100 * 1024 * 1024,
+    ...(silenceStderr ? { stdio: ['ignore', 'pipe', 'pipe'] as const } : {}),
   }).trim();
 }
 
@@ -951,10 +967,19 @@ function git(repoPath: string, args: string[], configs: string[] = [], timeoutMs
  * `git -C <path> rev-parse --show-toplevel`. Handles worktrees and submodules
  * natively (git itself resolves them). Throws a user-friendly error when no
  * git repo is found.
+ *
+ * The probe's failure is expected and routine (a non-git-yet brain dir, a
+ * scratch dir, a caller checking "is this a repo?") — `sync.ts` self-heals
+ * it (git-init) or surfaces the message below, never the raw git stderr.
+ * `silenceStderr: true` keeps git's own `fatal: not a git repository ...`
+ * off the process's real stderr so operator log-scanning for `fatal:` as a
+ * crash signature doesn't false-alarm on every routine probe miss (#2964
+ * auto-recovery made the *outcome* self-healing; this keeps the *log* quiet
+ * about the expected miss that triggered it).
  */
 export function discoverGitRoot(inputPath: string): string {
   try {
-    return git(inputPath, ['rev-parse', '--show-toplevel']);
+    return git(inputPath, ['rev-parse', '--show-toplevel'], [], 30000, { silenceStderr: true });
   } catch {
     throw new Error(
       `Not inside a git repository: ${inputPath}. GBrain sync requires a git-initialized repo (or a subdirectory of one).`,

test/sync-discover-git-root-stderr.test.ts

@@ -0,0 +1,84 @@
+/**
+ * discoverGitRoot's `rev-parse --show-toplevel` probe fails routinely (a
+ * scratch dir, a not-yet-git-initialized brain dir, `#2964` auto-recovery's
+ * own probe). Node's `execFileSync` writes the child's stderr straight
+ * through to the parent's real stderr by default, so every one of these
+ * *expected* misses used to dump git's raw `fatal: not a git repository
+ * (or any of the parent directories): .git` line onto gbrain's own stderr —
+ * indistinguishable, to an operator grepping logs for `fatal:` as a crash
+ * signature, from an actual crash. `discoverGitRoot` already handles the
+ * failure (throws a friendlier `Error`, or `sync.ts`'s `#2964` auto-recovery
+ * catches it and git-inits); only the process-level stderr leak was the bug.
+ *
+ * These tests spawn a real `bun` subprocess (rather than monkeypatching
+ * `process.stderr.write` in-process) because the leak happens at the OS file
+ * descriptor level — `execFileSync`'s default `stdio: 'inherit'`-for-stderr
+ * behavior writes directly to fd 2 of whichever process calls it, bypassing
+ * `process.stderr.write()` entirely. A subprocess is the only way to observe
+ * (or fail to observe) that write from the test.
+ */
+
+import { describe, test, expect } from 'bun:test';
+import { spawnSync } from 'node:child_process';
+import { mkdtempSync, rmSync } from 'fs';
+import { join, basename } from 'path';
+import { tmpdir } from 'os';
+
+const SYNC_MODULE = join(import.meta.dir, '..', 'src', 'commands', 'sync.ts');
+
+function runDiscoverGitRootProbe(targetDir: string) {
+  return spawnSync(
+    'bun',
+    [
+      '-e',
+      `
+      import { discoverGitRoot } from ${JSON.stringify(SYNC_MODULE)};
+      try {
+        // Success path prints to STDOUT only — stderr must stay untouched
+        // by both the probe itself and this harness so the tests can make
+        // a clean "nothing on stderr" assertion.
+        console.log('OK:' + discoverGitRoot(${JSON.stringify(targetDir)}));
+      } catch (e) {
+        console.error('CAUGHT:' + e.message);
+      }
+      `,
+    ],
+    { encoding: 'utf-8', env: { ...process.env, NO_COLOR: '1' } },
+  );
+}
+
+describe('discoverGitRoot probe stderr hygiene', () => {
+  test('an expected probe failure (non-git dir) never leaks git\'s raw "fatal:" line to stderr', () => {
+    const dir = mkdtempSync(join(tmpdir(), 'gbrain-nogit-'));
+    try {
+      const res = runDiscoverGitRootProbe(dir);
+      expect(res.status).toBe(0);
+      // The caller's own friendly error still surfaces...
+      expect(res.stderr).toContain('CAUGHT:Not inside a git repository');
+      // ...but git's own raw stderr line must not reach the process stderr.
+      expect(res.stderr).not.toMatch(/fatal:/i);
+      expect(res.stderr).not.toContain('not a git repository (or any of the parent directories)');
+    } finally {
+      rmSync(dir, { recursive: true, force: true });
+    }
+  });
+
+  test('a successful probe (real git repo) still returns the toplevel and prints nothing to stderr', () => {
+    const dir = mkdtempSync(join(tmpdir(), 'gbrain-git-'));
+    try {
+      spawnSync('git', ['init', '--quiet'], { cwd: dir });
+      const res = runDiscoverGitRootProbe(dir);
+      expect(res.status).toBe(0);
+      // The resolved toplevel path lands on stdout, proving the probe
+      // actually succeeded (not just "didn't throw"). Compare by basename
+      // only — macOS resolves `/tmp`/`/var` symlinks (e.g. to
+      // `/private/tmp/...`), so the raw `dir` string may not appear verbatim
+      // in git's `--show-toplevel` output even on a clean success.
+      expect(res.stdout).toContain('OK:');
+      expect(res.stdout).toContain(basename(dir));
+      expect(res.stderr).toBe('');
+    } finally {
+      rmSync(dir, { recursive: true, force: true });
+    }
+  });
+});