From 2f2dfd341043587d57ce8da4c87790d3d757942d Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 21 Jul 2026 14:48:28 -0700 Subject: [PATCH] fix(gbrain): disable hooks for scaffolding commit to avoid harden push race MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Takeover of #2545. The harden flow installs the durability post-commit hook, then commitScaffolding commits + pushes; the hook fires on that commit and background-pushes (pull --rebase -> index.lock), racing the foreground push. #2545 added --no-verify, but that only skips pre-commit/commit-msg hooks — post-commit still runs (empirically verified). Fix: run the scaffolding commit with -c core.hooksPath=/dev/null so no hooks fire, and correct the hook template's misleading 'Bypass: git commit --no-verify' comment. Co-authored-by: peterostrander2 Co-Authored-By: Claude Fable 5 --- src/core/brain-repo-durability.ts | 8 ++++++-- test/brain-repo-durability.serial.test.ts | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/core/brain-repo-durability.ts b/src/core/brain-repo-durability.ts index 8ac06df21..28c94cec0 100644 --- a/src/core/brain-repo-durability.ts +++ b/src/core/brain-repo-durability.ts @@ -156,7 +156,8 @@ function renderPostCommitHook(): string { ${HOOK_BANNER} # LOCAL + untracked — NEVER commit this file. Best-effort background auto-push so # agent writes don't sit local-only. The real guarantee is ${HELPER_REL}. -# Bypass: git commit --no-verify. +# Bypass: git -c core.hooksPath=/dev/null commit … (--no-verify only skips +# pre-commit/commit-msg hooks; post-commit hooks still run under it). set -euo pipefail _branch="$(git rev-parse --abbrev-ref HEAD 2>/dev/null || echo HEAD)" @@ -729,7 +730,10 @@ function commitScaffolding(repoPath: string, branch: string, redact: (s: string) stdio: ['ignore', 'pipe', 'ignore'], timeout: 10_000, env: { ...process.env, ...GIT_ENV }, }).toString().trim(); if (!staged) return { status: 'ok', detail: 'scaffolding already committed' }; - execFileSync('git', ['-C', repoPath, 'commit', '-m', 'chore(gbrain): install brain durability scaffolding'], { + // #2545: commit with hooks disabled so the just-installed post-commit hook + // doesn't background-push (pull --rebase → index.lock) while we push below. + // NOTE: --no-verify would NOT do this — it skips pre-commit/commit-msg only. + execFileSync('git', ['-C', repoPath, '-c', 'core.hooksPath=/dev/null', 'commit', '-m', 'chore(gbrain): install brain durability scaffolding'], { stdio: 'ignore', timeout: 30_000, env: { ...process.env, ...GIT_ENV }, }); execFileSync('git', ['-C', repoPath, ...['-c', 'http.followRedirects=false'], 'push', 'origin', `HEAD:${branch}`], { diff --git a/test/brain-repo-durability.serial.test.ts b/test/brain-repo-durability.serial.test.ts index d381e492b..301347af2 100644 --- a/test/brain-repo-durability.serial.test.ts +++ b/test/brain-repo-durability.serial.test.ts @@ -163,6 +163,21 @@ describe('hardenBrainRepo', () => { expect(r.steps.find(s => s.step === 'commit')).toBeUndefined(); }); + test('#2545 — scaffolding commit does not fire the post-commit hook (no push race)', async () => { + await harden(); + // The gbrain hook's brain_push always writes to $GBRAIN_HOME/brain-push.log + // (ok / rejected / lock-timeout — every path logs). If the scaffolding + // commit fired the hook, the detached push races commitScaffolding's own + // push (pull --rebase → index.lock) and this log appears within ms. + await new Promise(r => setTimeout(r, 1500)); + expect(existsSync(join(process.env.GBRAIN_HOME!, 'brain-push.log'))).toBe(false); + // And the hook no longer documents a bypass that doesn't work: --no-verify + // skips pre-commit/commit-msg only, never post-commit. + const hook = readFileSync(join(work, '.git', 'hooks', 'post-commit'), 'utf-8'); + expect(hook).not.toContain('Bypass: git commit --no-verify'); + expect(hook).toContain('core.hooksPath=/dev/null'); + }); + test('dry-run makes no commit and writes no files', async () => { const before = commitCount(work); await harden({ dryRun: true });