From 546c585f76066ec102cbfd09494a80bb12b482d6 Mon Sep 17 00:00:00 2001 From: "A. Mahoney-Fernandes" <83057795+0xPlayerOne@users.noreply.github.com> Date: Thu, 1 Oct 2026 15:20:34 -0400 Subject: [PATCH 1/3] fix(hooks): run the gate from a local dependency, not npx The 1.39.2 hook shelled to `npx --yes code-foundry@` on every commit. Two problems, both observed rather than anticipated: - The gate's exit status was discarded. `if npx ...; then exit 0; fi` followed by an unconditional `git diff --cached --check` meant a failing gate fell through to the fallback and the commit went through. In this repository a commit carrying an unused import passed the hook, which printed the oxlint finding and landed anyway. - `fleet-manifest.mjs` installs `core.hooksPath` and makes the hook executable in every managed consumer worktree, so the npx call ran on every commit in every fleet worktree, and broke the fleet upgrade test. The hook now prefers a runtime the repository already depends on: `node_modules/.bin/code-foundry`, then a vendored `src/runtime.mjs`. Nothing is fetched at commit time. A repository opts into the gate by depending on code-foundry; one that does not keeps the whitespace guard and stays offline. The fallback is last, so a failing gate can no longer be swallowed. Four tests, all hermetic: no npx or registry reference; a depended-on gate that exits 7 fails the commit; no dependency falls back to the whitespace guard; and that guard still blocks trailing whitespace. --- .githooks/pre-commit | 11 +++- src/commands/sync.mjs | 17 ------ test/sync-pre-commit-hook.test.mjs | 92 ++++++++++++++++-------------- 3 files changed, 59 insertions(+), 61 deletions(-) diff --git a/.githooks/pre-commit b/.githooks/pre-commit index dc615cf..6f4b424 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -6,8 +6,15 @@ if [ -z "$(git diff --cached --name-only)" ]; then exit 0 fi -if command -v npx >/dev/null 2>&1 && npx --yes code-foundry@__VERSION__ pre-commit; then - exit 0 +# Prefer a runtime the repository already depends on. A repository opts into +# the gate by depending on code-foundry; one that does not keeps the +# whitespace guard and stays offline. Nothing is fetched at commit time. +if [ -x node_modules/.bin/code-foundry ]; then + exec node_modules/.bin/code-foundry pre-commit +fi + +if [ -f src/runtime.mjs ]; then + exec node src/runtime.mjs pre-commit fi git diff --cached --check diff --git a/src/commands/sync.mjs b/src/commands/sync.mjs index d4da271..3464d10 100644 --- a/src/commands/sync.mjs +++ b/src/commands/sync.mjs @@ -260,9 +260,6 @@ function synchronize(options) { if (file === 'LICENSE' && license !== 'preserve' && license !== 'none') continue if (file === '.github/CODEOWNERS' && existsSync(destination)) continue let content = readFileSync(sourceFile) - if (file === '.githooks/pre-commit') { - content = Buffer.from(renderHook(sourceFile, source)) - } if (file === 'release-please-config.json') { content = Buffer.from(renderReleaseConfig(target, sourceFile)) } @@ -589,20 +586,6 @@ function renderReleaseConfig(target, sourceFile) { return `${JSON.stringify(mergeReleaseConfig(target, sourceFile), null, 2)}\n` } -/** - * The hook runs `npx code-foundry@` on every commit, so the version - * is substituted at generation time rather than resolved at run time. A - * mutable dist-tag there would let any publish under that tag run code on - * every developer's machine at commit time; pinning means the gate a - * repository received is the gate that generated it. - * - * @param {string} sourceFile the template path - * @param {string} sourceRoot the package root that carries package.json - */ -function renderHook(sourceFile, sourceRoot) { - return readFileSync(sourceFile, 'utf8').replaceAll('__VERSION__', readPackageVersion(sourceRoot)) -} - /** @param {string} file @param {string} languages @param {string} features @param {Record} config */ function shouldInclude(file, languages, features, config) { if (file === 'ruff.toml') return includesValue(languages, 'python') diff --git a/test/sync-pre-commit-hook.test.mjs b/test/sync-pre-commit-hook.test.mjs index ca5b90b..526b717 100644 --- a/test/sync-pre-commit-hook.test.mjs +++ b/test/sync-pre-commit-hook.test.mjs @@ -1,61 +1,69 @@ import { strict as assert } from 'node:assert' -import { mkdtempSync, readFileSync, rmSync, writeFileSync, mkdirSync } from 'node:fs' +import { mkdtempSync, readFileSync, rmSync, writeFileSync, mkdirSync, chmodSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { describe, it } from 'node:test' -import { syncRepository, readPackageVersion } from '../src/commands/sync.mjs' +import { spawnSync } from 'node:child_process' +import { syncRepository } from '../src/commands/sync.mjs' function consumer() { - const root = mkdtempSync(join(tmpdir(), 'code-foundry-hook-pin-')) + const root = mkdtempSync(join(tmpdir(), 'cf-hook-')) mkdirSync(join(root, '.github/workflows'), { recursive: true }) - writeFileSync(join(root, 'package.json'), '{"name":"fixture","version":"1.0.0"}\n') - writeFileSync( - join(root, '.github/code-foundry.yml'), - 'languages: typescript\npackage_manager: bun\nfeatures: all\ngit_workflow: direct\nmerge_strategy: squash\nrelease_merge_strategy: squash\n' - ) + writeFileSync(join(root, 'package.json'), '{"name":"f","version":"1.0.0"}\n') + writeFileSync(join(root, '.github/code-foundry.yml'), + 'languages: typescript\npackage_manager: bun\nfeatures: all\ngit_workflow: direct\nmerge_strategy: squash\nrelease_merge_strategy: squash\n') return root } +function stage(root) { + writeFileSync(join(root, 'a.txt'), 'x\n') + spawnSync('git', ['init', '-q'], { cwd: root }) + spawnSync('git', ['add', 'a.txt'], { cwd: root }) +} +function gate(root) { + const p = join(root, '.githooks/pre-commit') + chmodSync(p, 0o755) + return spawnSync(p, [], { cwd: root, encoding: 'utf8' }) +} + +describe('generated pre-commit hook', () => { + it('never fetches anything at commit time', () => { + const root = consumer(); try { + syncRepository({ target: root, source: process.cwd() }) + const h = readFileSync(join(root, '.githooks/pre-commit'), 'utf8') + assert.doesNotMatch(h, /\bnpx\b/, 'must not shell out to npx') + assert.doesNotMatch(h, /https?:/, 'must not reference a registry') + } finally { rmSync(root, { recursive: true, force: true }) } + }) -describe('Generated pre-commit hook', () => { - it('pins the gate to the version that generated it', () => { - const root = consumer() - try { + it('fails the commit when the depended-on gate fails', () => { + const root = consumer(); try { syncRepository({ target: root, source: process.cwd() }) - const hook = readFileSync(join(root, '.githooks/pre-commit'), 'utf8') - const version = readPackageVersion(process.cwd()) - assert.ok( - hook.includes(`code-foundry@${version} pre-commit`), - `expected the gate pinned to ${version}, got: ${hook.split('\n').find((l) => l.includes('npx')) ?? ''}` - ) - } finally { - rmSync(root, { recursive: true, force: true }) - } + mkdirSync(join(root, 'node_modules/.bin'), { recursive: true }) + writeFileSync(join(root, 'node_modules/.bin/code-foundry'), '#!/bin/sh\nexit 7\n') + chmodSync(join(root, 'node_modules/.bin/code-foundry'), 0o755) + stage(root) + const r = gate(root) + assert.equal(r.status, 7, `gate failure must fail the commit, got ${r.status}`) + } finally { rmSync(root, { recursive: true, force: true }) } }) - it('never resolves the gate through a mutable dist-tag', () => { - const root = consumer() - try { + it('falls back to the whitespace guard when the gate is not installed', () => { + const root = consumer(); try { syncRepository({ target: root, source: process.cwd() }) - const hook = readFileSync(join(root, '.githooks/pre-commit'), 'utf8') - // A dist-tag here would let any publish under that tag execute code on - // every developer's machine at commit time. - assert.doesNotMatch(hook, /code-foundry@(latest|next|beta|canary)\b/) - assert.doesNotMatch(hook, /__VERSION__/) - } finally { - rmSync(root, { recursive: true, force: true }) - } + stage(root) + const r = gate(root) + assert.equal(r.status, 0) + } finally { rmSync(root, { recursive: true, force: true }) } }) - it('keeps the whitespace guard as an offline fallback', () => { - const root = consumer() - try { + it('blocks a commit with trailing whitespace only when the gate is absent', () => { + const root = consumer(); try { syncRepository({ target: root, source: process.cwd() }) - const hook = readFileSync(join(root, '.githooks/pre-commit'), 'utf8') - assert.match(hook, /git diff --cached --check/) - // The fallback must not be able to short-circuit the gate. - assert.match(hook, /exit 0[\s\S]*npx --yes code-foundry@\d+\.\d+\.\d+ pre-commit/) - } finally { - rmSync(root, { recursive: true, force: true }) - } + writeFileSync(join(root, 'b.txt'), 'trailing \n') + spawnSync('git', ['init', '-q'], { cwd: root }) + spawnSync('git', ['add', 'b.txt'], { cwd: root }) + const r = gate(root) + assert.notEqual(r.status, 0, 'the fallback must still catch whitespace errors') + } finally { rmSync(root, { recursive: true, force: true }) } }) }) From 1b2379726396926f931d41eb5c2de3c670c94afc Mon Sep 17 00:00:00 2001 From: "A. Mahoney-Fernandes" <83057795+0xPlayerOne@users.noreply.github.com> Date: Thu, 1 Oct 2026 15:21:38 -0400 Subject: [PATCH 2/3] chore: re-trigger validation From 5074980d131f4c91cf7ee1467f9224d9e4b0af46 Mon Sep 17 00:00:00 2001 From: "A. Mahoney-Fernandes" <83057795+0xPlayerOne@users.noreply.github.com> Date: Thu, 1 Oct 2026 15:30:31 -0400 Subject: [PATCH 3/3] chore: format the pre-commit hook test --- test/sync-pre-commit-hook.test.mjs | 34 +++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/test/sync-pre-commit-hook.test.mjs b/test/sync-pre-commit-hook.test.mjs index 526b717..c0cb981 100644 --- a/test/sync-pre-commit-hook.test.mjs +++ b/test/sync-pre-commit-hook.test.mjs @@ -10,8 +10,10 @@ function consumer() { const root = mkdtempSync(join(tmpdir(), 'cf-hook-')) mkdirSync(join(root, '.github/workflows'), { recursive: true }) writeFileSync(join(root, 'package.json'), '{"name":"f","version":"1.0.0"}\n') - writeFileSync(join(root, '.github/code-foundry.yml'), - 'languages: typescript\npackage_manager: bun\nfeatures: all\ngit_workflow: direct\nmerge_strategy: squash\nrelease_merge_strategy: squash\n') + writeFileSync( + join(root, '.github/code-foundry.yml'), + 'languages: typescript\npackage_manager: bun\nfeatures: all\ngit_workflow: direct\nmerge_strategy: squash\nrelease_merge_strategy: squash\n' + ) return root } function stage(root) { @@ -27,16 +29,20 @@ function gate(root) { describe('generated pre-commit hook', () => { it('never fetches anything at commit time', () => { - const root = consumer(); try { + const root = consumer() + try { syncRepository({ target: root, source: process.cwd() }) const h = readFileSync(join(root, '.githooks/pre-commit'), 'utf8') assert.doesNotMatch(h, /\bnpx\b/, 'must not shell out to npx') assert.doesNotMatch(h, /https?:/, 'must not reference a registry') - } finally { rmSync(root, { recursive: true, force: true }) } + } finally { + rmSync(root, { recursive: true, force: true }) + } }) it('fails the commit when the depended-on gate fails', () => { - const root = consumer(); try { + const root = consumer() + try { syncRepository({ target: root, source: process.cwd() }) mkdirSync(join(root, 'node_modules/.bin'), { recursive: true }) writeFileSync(join(root, 'node_modules/.bin/code-foundry'), '#!/bin/sh\nexit 7\n') @@ -44,26 +50,34 @@ describe('generated pre-commit hook', () => { stage(root) const r = gate(root) assert.equal(r.status, 7, `gate failure must fail the commit, got ${r.status}`) - } finally { rmSync(root, { recursive: true, force: true }) } + } finally { + rmSync(root, { recursive: true, force: true }) + } }) it('falls back to the whitespace guard when the gate is not installed', () => { - const root = consumer(); try { + const root = consumer() + try { syncRepository({ target: root, source: process.cwd() }) stage(root) const r = gate(root) assert.equal(r.status, 0) - } finally { rmSync(root, { recursive: true, force: true }) } + } finally { + rmSync(root, { recursive: true, force: true }) + } }) it('blocks a commit with trailing whitespace only when the gate is absent', () => { - const root = consumer(); try { + const root = consumer() + try { syncRepository({ target: root, source: process.cwd() }) writeFileSync(join(root, 'b.txt'), 'trailing \n') spawnSync('git', ['init', '-q'], { cwd: root }) spawnSync('git', ['add', 'b.txt'], { cwd: root }) const r = gate(root) assert.notEqual(r.status, 0, 'the fallback must still catch whitespace errors') - } finally { rmSync(root, { recursive: true, force: true }) } + } finally { + rmSync(root, { recursive: true, force: true }) + } }) })