diff --git a/lib/ci-audit/static.ts b/lib/ci-audit/static.ts index 01d25f53c..701cf572c 100644 --- a/lib/ci-audit/static.ts +++ b/lib/ci-audit/static.ts @@ -71,21 +71,29 @@ export function parseWorkflow(file: string, text: string, defaultBranch: string) }; } -function readHook(root: string): HookInfo | null { +const hookRuns = (text: string) => ['lint', 'typecheck', 'format', 'test'].filter(k => new RegExp(k === 'typecheck' ? 'typecheck|tsc|tsgo|type-check' : k, 'i').test(text)); + +/** The pre-push hook: local git config first, then tracked sources a fresh CI checkout also has. */ +function readHook(root: string, files: string[], pkg: any): HookInfo | null { const hooksPath = git(root, ['config', '--get', 'core.hooksPath']); + const hooksDir = git(root, ['rev-parse', '--git-path', 'hooks']); const candidates: [string, string][] = [ - ...(hooksPath ? [[ 'core.hooksPath', path.join(root, hooksPath, 'pre-push') ] as [string, string]] : []), + ...(hooksPath ? [['core.hooksPath', path.join(root, hooksPath, 'pre-push')] as [string, string]] : []), ['husky', path.join(root, '.husky', 'pre-push')], - ['lefthook', path.join(root, 'lefthook.yml')], - ['git', path.join(git(root, ['rev-parse', '--git-path', 'hooks']) ? path.resolve(root, git(root, ['rev-parse', '--git-path', 'hooks'])!) : path.join(root, '.git', 'hooks'), 'pre-push')], + ...['lefthook.yml', 'lefthook.yaml', '.lefthook.yml', '.lefthook.yaml'].map(f => ['lefthook', path.join(root, f)] as [string, string]), + ['pre-commit', path.join(root, '.pre-commit-config.yaml')], + ['git', path.join(hooksDir ? path.resolve(root, hooksDir) : path.join(root, '.git', 'hooks'), 'pre-push')], + ...files.filter(f => path.basename(f) === 'pre-push').map(f => ['tracked', path.join(root, f)] as [string, string]), ]; for (const [manager, file] of candidates) { if (!fs.existsSync(file)) continue; const text = fs.readFileSync(file, 'utf8'); if (manager === 'lefthook' && !/pre-push:/.test(text)) continue; - const runs = ['lint', 'typecheck', 'format', 'test'].filter(k => new RegExp(k === 'typecheck' ? 'typecheck|tsc|tsgo|type-check' : k, 'i').test(text)); - return { manager, file: path.relative(root, file), runs }; + if (manager === 'pre-commit' && !/pre-push/.test(text)) continue; + return { manager, file: path.relative(root, file), runs: hookRuns(text) }; } + const simple = pkg?.['simple-git-hooks']?.['pre-push']; + if (typeof simple === 'string') return { manager: 'simple-git-hooks', file: 'package.json', runs: hookRuns(simple) }; return null; } @@ -99,7 +107,7 @@ export function readStatic(root: string, defaultBranch: string): StaticView { return { root, head: git(root, ['rev-parse', 'HEAD']) ?? 'unknown', dirty: Boolean(git(root, ['status', '--porcelain'])), workflows, packageScripts: pkg.scripts ?? {}, deps: Object.keys({ ...(pkg.dependencies ?? {}), ...(pkg.devDependencies ?? {}) }), - files, hook: readHook(root), + files, hook: readHook(root, files, pkg), testFilesMentioning: (pattern: RegExp) => testFiles.filter(f => { try { return pattern.test(fs.readFileSync(path.join(root, f), 'utf8')); } catch { return false; } }), }; } diff --git a/test/ci-audit.test.ts b/test/ci-audit.test.ts index d414198a0..dad0ef5c0 100644 --- a/test/ci-audit.test.ts +++ b/test/ci-audit.test.ts @@ -8,7 +8,7 @@ import { Budget } from '../lib/ci-audit/github'; import { concurrencyFlag, lessonRunners } from '../lib/ci-audit/lessons'; import { cohort, measure, phases } from '../lib/ci-audit/measure'; import { priceRunner } from '../lib/ci-audit/prices'; -import { ISSUE_MARKER, ISSUE_TITLE, findIssue, planIssue } from '../lib/ci-audit/publish'; +import { ISSUE_MARKER, ISSUE_TITLE, applyIssue, findIssue, planIssue } from '../lib/ci-audit/publish'; import type { AuditReport } from '../lib/ci-audit/report'; import { CI_SHAPING, MIN_COHORT, comparable, judgeHead, staticChecks } from '../lib/ci-audit/ship-check'; import { parseWorkflow, readStatic } from '../lib/ci-audit/static'; @@ -225,6 +225,23 @@ describe('cost accounting inputs', () => { }); }); +describe('pre-push hook in a fresh checkout', () => { + const repo = (files: Record) => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'ci-audit-hook-')); + for (const [f, text] of Object.entries(files)) { fs.mkdirSync(path.dirname(path.join(dir, f)), { recursive: true }); fs.writeFileSync(path.join(dir, f), text); } + for (const args of [['init', '-q'], ['add', '-A']]) Bun.spawnSync(['git', ...args], { cwd: dir }); + return dir; + }; + test('a tracked hook directory, every lefthook spelling, simple-git-hooks and pre-commit count without local git config', () => { + expect(readStatic(repo({ '.githooks/pre-push': 'bun run lint && bun run typecheck', 'package.json': '{"scripts":{"prepare":"git config core.hooksPath .githooks"}}' }), 'main').hook).toMatchObject({ file: '.githooks/pre-push', runs: ['lint', 'typecheck'] }); + expect(readStatic(repo({ 'lefthook.yaml': 'pre-push:\n commands:\n lint: { run: eslint }' }), 'main').hook?.manager).toBe('lefthook'); + expect(readStatic(repo({ '.lefthook.yml': 'pre-push:\n commands:\n tc: { run: tsc }' }), 'main').hook?.runs).toEqual(['typecheck']); + expect(readStatic(repo({ 'package.json': '{"simple-git-hooks":{"pre-push":"npm run lint"}}' }), 'main').hook).toMatchObject({ manager: 'simple-git-hooks', runs: ['lint'] }); + expect(readStatic(repo({ '.pre-commit-config.yaml': 'repos:\n- repo: local\n hooks:\n - id: lint\n entry: ruff\n stages: [pre-push]' }), 'main').hook?.manager).toBe('pre-commit'); + expect(readStatic(repo({ 'README.md': 'x' }), 'main').hook).toBeNull(); + }); +}); + describe('ship-check static rules', () => { test('CI-shaping files include workflows, package.json, hook managers and linter configs, not ordinary source', () => { for (const f of ['.github/workflows/ci.yml', 'package.json', 'web/package.json', 'bunfig.toml', 'tsconfig.json', '.husky/pre-push', 'lefthook.yml', 'biome.json', 'vitest.config.ts', '.gstack/pregate.json']) expect(CI_SHAPING.test(f)).toBe(true); @@ -290,6 +307,20 @@ describe('weekly issue', () => { expect(planIssue({ exit: 4, report: { ...withFixes, partial: ['x'] } as AuditReport }, null).action).toBe('create'); }); + test('each issue action writes to the right issue in order, and a refused write throws', async () => { + const calls: string[] = []; + const write = async (m: string, p: string, b: Record) => { calls.push(m + ' ' + p + ' ' + Object.keys(b).join(',')); return 201; }; + const ex = { number: 7, body: '' }; + expect(await applyIssue(write, 'o/r', { action: 'create', body: 'b' }, null)).toBe('CI_AUDIT_ISSUE: created'); + expect(await applyIssue(write, 'o/r', { action: 'update', body: 'b' }, ex)).toBe('CI_AUDIT_ISSUE: updated #7'); + expect(await applyIssue(write, 'o/r', { action: 'keep', comment: 'c' }, ex)).toBe('CI_AUDIT_ISSUE: kept #7 open'); + expect(await applyIssue(write, 'o/r', { action: 'close', comment: 'c' }, ex)).toBe('CI_AUDIT_ISSUE: closed #7'); + expect(await applyIssue(write, 'o/r', { action: 'none', reason: 'r' }, null)).toBe('CI_AUDIT_ISSUE: none (r)'); + expect(calls).toEqual(['POST /repos/o/r/issues title,body', 'PATCH /repos/o/r/issues/7 body', 'POST /repos/o/r/issues/7/comments body', + 'POST /repos/o/r/issues/7/comments body', 'PATCH /repos/o/r/issues/7 state,state_reason']); + await expect(applyIssue(async () => 422, 'o/r', { action: 'create', body: 'b' }, null)).rejects.toThrow('ci_audit_api_failed'); + }); + test('job names from a fork PR cannot break the tables or mention anyone', () => { const t = { p50: 1, p90: 2 }; const body = planIssue({ exit: 0, report: { ...withFixes, jobs: [{ key: 'x | [y](https://e.example) @team o/r#1', runner: 'ubuntu-latest', n: 1, wait: t, setup: t, run: t, billedMinutesPerWeek: 1, usdPerMonth: 0 }] } as unknown as AuditReport }, null) as { body: string };