diff --git a/docs/ci-audit.md b/docs/ci-audit.md index 88ad9c301..877442af9 100644 --- a/docs/ci-audit.md +++ b/docs/ci-audit.md @@ -68,7 +68,8 @@ are unknown. `--record ` saves the GitHub responses (allowlisted fields onl ## The /ship check -`ship-check` runs on every `/ship`. It takes a few seconds, costs no model calls or +`ship-check` is the check `/ship` will run on every ship (the `/ship` step that calls it +lands in a follow-up PR; until then, run it by hand). It takes a few seconds, costs no model calls or dollars, and never blocks a ship. With CI-shaping files in the diff (workflows, `package.json`, hook and linter configs) it checks them against the lessons. With a PR number it compares the PR's previous settled push with up to 40 comparable all-green PR diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index eac6bd580..dcb0a61ea 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -5695,7 +5695,7 @@ Every error is one line, `CI_AUDIT: unavailable ()`, followed by ### `CI_AUDIT: unavailable of GitHub calls left this hour (ci_audit_rate_limit)` -**Meaning.** The preflight found fewer GitHub API calls left this hour than the run could use plus 500 spare, so it stopped before spending any. On /ship this line is informational: the ship continues and nothing is blocked. **Fix.** Wait until the reset time printed on the retry line, or lower --max-calls. +**Meaning.** The preflight found fewer GitHub API calls left this hour than the run could use plus a spare tenth of the hour's limit (at most 500), so it stopped before spending any. On /ship this line is informational: the ship continues and nothing is blocked. **Fix.** Wait until the reset time printed on the retry line, or lower --max-calls. ### `CI_AUDIT: unavailable call budget of spent (ci_audit_call_budget)` diff --git a/lib/ci-audit/collect.ts b/lib/ci-audit/collect.ts index 932a84f39..7595d0d9f 100644 --- a/lib/ci-audit/collect.ts +++ b/lib/ci-audit/collect.ts @@ -3,7 +3,7 @@ import type { LogEvidence } from './lessons'; export interface Step { name: string; started_at: string | null; completed_at: string | null; conclusion: string | null } export interface Job { id: number; run_id: number; name: string; status: string; conclusion: string | null; created_at: string; started_at: string | null; completed_at: string | null; labels: string[]; steps: Step[] } -export interface Run { id: number; name: string; path: string; event: string; head_sha: string; head_branch: string; status: string; conclusion: string | null; created_at: string; run_started_at?: string; updated_at: string; run_attempt: number; pull_requests?: { number: number; base?: { ref?: string } }[] } +export interface Run { id: number; name: string; path: string; event: string; head_sha: string; head_branch: string; status: string; conclusion: string | null; created_at: string; run_started_at?: string; updated_at: string; run_attempt: number; head_repository?: { full_name?: string }; pull_requests?: { number: number; base?: { ref?: string } }[] } export interface Cache { key: string; ref: string; size_in_bytes: number; last_accessed_at: string } export interface AuditData { @@ -31,7 +31,7 @@ export async function preflight(client: ApiClient, budget: Budget): Promise<{ li const res = await client.get('/rate_limit'); const core = (res.json as any)?.resources?.core; if (res.status !== 200 || !core) return null; - const need = budget.maxCalls - budget.used + RATE_HEADROOM; + const need = budget.maxCalls - budget.used + Math.min(RATE_HEADROOM, Math.floor(core.limit / 10)); if (core.remaining < need) { const reset = new Date(core.reset * 1000).toISOString(); throw new AuditError('ci_audit_rate_limit', `${core.remaining} of ${core.limit} GitHub calls left this hour; this run needs up to ${need}`, `after ${reset}`); diff --git a/lib/ci-audit/report.ts b/lib/ci-audit/report.ts index 5a79a2cf0..308e082bd 100644 --- a/lib/ci-audit/report.ts +++ b/lib/ci-audit/report.ts @@ -59,6 +59,9 @@ export function buildReport(data: AuditData, m: Measures, ls: Lesson[], st: Stat const t = (s: number | null) => (s == null ? '-' : s >= 120 ? `${(s / 60).toFixed(1)}m` : `${Math.round(s)}s`); const STATE = { pass: 'pass', finding: 'FINDING', unknown: 'unknown', not_applicable: 'n/a' } as const; +/** Job and workflow names come from workflow files a fork PR controls: no table breaks, links, HTML or mentions. */ +const esc = (s: string) => String(s).replace(/[\\|<>[\]`]/g, c => `\\${c}`).replace(/@/g, '@\u200b'); + export function renderMarkdown(r: AuditReport, maxJobs = 12): string { const out: string[] = []; out.push(`# CI audit: ${r.repo}`, ''); @@ -67,17 +70,17 @@ export function renderMarkdown(r: AuditReport, maxJobs = 12): string { if (r.partial.length) out.push('', `**Partial:** ${r.partial.join('; ')}`); const ex = Object.entries(r.pushToGreen.excluded).filter(([, n]) => n).map(([k, n]) => `${n} ${k.replace(/_/g, ' ')}`).join(', '); out.push('', `**Push to all checks green:** p50 ${t(r.pushToGreen.p50)}, p90 ${t(r.pushToGreen.p90)} over ${r.pushToGreen.n} all-green PR heads${ex ? ` (left out: ${ex})` : ''}. Time to first failure p50 ${t(r.timeToFirstFailure.p50)} (n=${r.timeToFirstFailure.n}). Wait before the first job: p50 ${t(r.preJobWait.p50)}.`); - if (r.pushToGreen.bottlenecks.length) out.push(`Finished last: ${r.pushToGreen.bottlenecks.map(b => `${b.workflow} ${Math.round(b.share * 100)}%`).join(', ')}.`); + if (r.pushToGreen.bottlenecks.length) out.push(`Finished last: ${r.pushToGreen.bottlenecks.map(b => `${esc(b.workflow)} ${Math.round(b.share * 100)}%`).join(', ')}.`); out.push('', '## Ranked fixes', ''); if (!r.fixes.length) out.push('No lesson has a finding.'); r.fixes.forEach((f, i) => { const gain = [f.prMinutes != null ? `up to ${f.prMinutes} min off each PR${f.measured ? ' (measured)' : ' (estimated)'}` : null, f.jobMinutesPerWeek != null ? `${f.jobMinutesPerWeek} runner-min/week` : null, f.usdPerMonth != null ? `${f.usdPerMonth >= 0 ? '+' : ''}$${f.usdPerMonth}/month` : null].filter(Boolean).join(', '); - out.push(`${i + 1}. **Lesson ${f.lesson}, ${f.name}.** ${f.summary}. Fix: ${f.fix}.${gain ? ` Gain: ${gain}.` : ''} ${f.note}.${f.caution ? ` Caution: ${f.caution}.` : ''}`); + out.push(`${i + 1}. **Lesson ${f.lesson}, ${f.name}.** ${esc(f.summary)}. Fix: ${esc(f.fix)}.${gain ? ` Gain: ${gain}.` : ''} ${esc(f.note)}.${f.caution ? ` Caution: ${esc(f.caution)}.` : ''}`); }); out.push('', '## The nine lessons', '', '| # | Lesson | State | What we saw |', '|---|---|---|---|'); - for (const l of r.lessons) out.push(`| ${l.id} | ${l.name} | ${STATE[l.state]} | ${l.summary}${l.evidence.length ? `
${l.evidence.slice(0, 3).join('
')}` : ''} |`); + for (const l of r.lessons) out.push(`| ${l.id} | ${l.name} | ${STATE[l.state]} | ${esc(l.summary)}${l.evidence.length ? `
${l.evidence.slice(0, 3).map(esc).join('
')}` : ''} |`); out.push('', '## Jobs (most runner time first)', '', '| Job | Runner | n | Wait p50/p90 | Setup p50 | Running p50 | Billed min/week | $/month |', '|---|---|---|---|---|---|---|---|'); - for (const j of r.jobs.slice(0, maxJobs)) out.push(`| ${j.key} | ${j.runner} | ${j.n} | ${t(j.wait.p50)}/${t(j.wait.p90)} | ${t(j.setup.p50)} | ${t(j.run.p50)} | ${j.billedMinutesPerWeek} | ${j.usdPerMonth} |`); + for (const j of r.jobs.slice(0, maxJobs)) out.push(`| ${esc(j.key)} | ${esc(j.runner)} | ${j.n} | ${t(j.wait.p50)}/${t(j.wait.p90)} | ${t(j.setup.p50)} | ${t(j.run.p50)} | ${j.billedMinutesPerWeek} | ${j.usdPerMonth} |`); out.push('', `**Cost:** about ${r.cost.billedMinutesPerWeek} billed runner-minutes a week, $${r.cost.usdPerMonth} a month at list prices (${r.cost.note}). Price rows and sources are in --json.`); return out.join('\n') + '\n'; } diff --git a/lib/ci-audit/ship-check.ts b/lib/ci-audit/ship-check.ts index faf7699b2..eb677db17 100644 --- a/lib/ci-audit/ship-check.ts +++ b/lib/ci-audit/ship-check.ts @@ -65,9 +65,11 @@ export async function timingCheck(client: ApiClient, repo: string, pr: number, m if (p.status !== 200) throw new AuditError('ci_audit_repo_unreadable', `cannot read PR #${pr} (HTTP ${p.status})`); const branch = String((p.json as any)?.head?.ref ?? ''); const base = String((p.json as any)?.base?.ref ?? ''); + const headRepo = (p.json as any)?.head?.repo?.full_name as string | undefined; const partial: string[] = []; const pages = [1, 2, 3, 4].map(n => listRuns(client, repo, 'event=pull_request', 1, partial, { hit: false }, { firstPage: n })); - const mine = await listRuns(client, repo, `event=pull_request&branch=${encodeURIComponent(branch)}`, 1, partial, { hit: false }); + const mine = (await listRuns(client, repo, `event=pull_request&branch=${encodeURIComponent(branch)}`, 1, partial, { hit: false })) + .filter(r => !headRepo || !r.head_repository?.full_name || r.head_repository.full_name === headRepo); const shas = [...new Set(mine.sort((a, b) => b.created_at.localeCompare(a.created_at)).map(r => r.head_sha))]; const settled = shas.map(s => headOf(mine, s)).find(Boolean) ?? null; if (!settled) return { state: 'pending', line: `CI_AUDIT: pending no settled all-green push on #${pr} yet` }; diff --git a/lib/ci-audit/templates/ci-audit-weekly.yml b/lib/ci-audit/templates/ci-audit-weekly.yml index 2e1fbb801..e9c7a01a5 100644 --- a/lib/ci-audit/templates/ci-audit-weekly.yml +++ b/lib/ci-audit/templates/ci-audit-weekly.yml @@ -27,28 +27,33 @@ jobs: timeout-minutes: 20 env: GH_TOKEN: ${{ github.token }} - # Pin a gstack tag or commit here to control upgrades. + # main follows gstack's latest; pin a release tag or a full commit sha to control upgrades. GSTACK_REF: main steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 with: filter: blob:none - - uses: oven-sh/setup-bun@v2 + - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2 - name: Fetch gstack - run: git clone --quiet --depth 1 --branch "$GSTACK_REF" https://github.com/garrytan/gstack "$RUNNER_TEMP/gstack" + run: | + git init --quiet "$RUNNER_TEMP/gstack" + git -C "$RUNNER_TEMP/gstack" fetch --quiet --depth 1 https://github.com/garrytan/gstack "$GSTACK_REF" + git -C "$RUNNER_TEMP/gstack" checkout --quiet FETCH_HEAD - name: Audit id: audit - # GITHUB_TOKEN allows 1,000 API calls an hour; the run refuses to start below its budget plus 500. + # GITHUB_TOKEN allows 1,000 API calls an hour; the run refuses to start below its budget plus 100. run: | set +e "$RUNNER_TEMP/gstack/bin/gstack-ci-audit" run --repo "$GITHUB_REPOSITORY" --since-days 7 --runs-per-workflow 10 --logs 5 --max-calls 400 --out audit.json > audit.md 2> audit.err echo "code=$?" >> "$GITHUB_OUTPUT" - cat audit.md audit.err + cat audit.err - name: Publish if: always() + env: + AUDIT_CODE: ${{ steps.audit.outputs.code || 5 }} run: | - "$RUNNER_TEMP/gstack/bin/gstack-ci-audit" publish --repo "$GITHUB_REPOSITORY" --exit "${{ steps.audit.outputs.code || 5 }}" --report audit.json --error-file audit.err --run-url "$GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID" - - uses: actions/upload-artifact@v4 + "$RUNNER_TEMP/gstack/bin/gstack-ci-audit" publish --repo "$GITHUB_REPOSITORY" --exit "$AUDIT_CODE" --report audit.json --error-file audit.err --run-url "$GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID" + - uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7 if: always() with: name: ci-audit diff --git a/test/ci-audit.test.ts b/test/ci-audit.test.ts index 1fbb319ff..1a12a30ad 100644 --- a/test/ci-audit.test.ts +++ b/test/ci-audit.test.ts @@ -3,7 +3,8 @@ import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; import { EXIT, main } from '../lib/ci-audit/cli'; -import type { Run } from '../lib/ci-audit/collect'; +import { preflight, type Run } from '../lib/ci-audit/collect'; +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'; @@ -264,6 +265,13 @@ describe('weekly issue', () => { expect(planIssue({ exit: 4, report: { ...withFixes, partial: ['x'] } as AuditReport }, null).action).toBe('create'); }); + 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 ', runner: 'ubuntu-latest', n: 1, wait: t, setup: t, run: t, billedMinutesPerWeek: 1, usdPerMonth: 0 }] } as unknown as AuditReport }, null) as { body: string }; + const row = body.body.split('\n').find(l => l.includes('e.example'))!; + expect(row).toBe('| x \\| \\[y\\](https://e.example) @\u200bteam \\ | ubuntu-latest | 1 | 1s/2s | 1s | 1s | 1 | 0 |'); + }); + test('finds only the workflow bot\'s own issue, past 100 open issues and an outsider\'s copy', async () => { const issue = (number: number, login: string, title = 'other', body = '') => ({ number, title, body, user: { login } }); const open = [issue(1, 'outsider', ISSUE_TITLE, `${ISSUE_MARKER}\nfake findings`), @@ -298,7 +306,15 @@ describe('Saturday workflow template', () => { expect(audit).toContain('set +e'); const pub = steps.find(s => s.name === 'Publish')!; expect(pub.if).toBe('always()'); - expect(pub.run).toContain('--exit "${{ steps.audit.outputs.code || 5 }}"'); + expect((pub as { env?: Record }).env?.AUDIT_CODE).toBe('${{ steps.audit.outputs.code || 5 }}'); + expect(pub.run).toContain('--exit "$AUDIT_CODE"'); + expect(steps.map(s => s.run ?? '').join('\n')).not.toContain('${{'); + }); + + test('a GITHUB_TOKEN with a little of its hour spent still starts the audit', async () => { + const left = (remaining: number) => ({ text: async () => null, get: async () => ({ status: 200, json: { resources: { core: { limit: 1000, remaining, reset: 1_800_000_000 } } }, headers: {} }) }); + expect(await preflight(left(550) as never, new Budget(400, 1000))).toMatchObject({ remaining: 550 }); + await expect(preflight(left(450) as never, new Budget(400, 1000))).rejects.toThrow('ci_audit_rate_limit'); }); }); diff --git a/test/egress-receipt-wiring.test.ts b/test/egress-receipt-wiring.test.ts index 6312ea3a5..210b485b0 100644 --- a/test/egress-receipt-wiring.test.ts +++ b/test/egress-receipt-wiring.test.ts @@ -169,6 +169,8 @@ const SCANNER_EXEMPT: Record = { 'served-page JS talking to its own loopback server (same-origin relative fetch)', 'design/src/compare.ts': 'served-page JS talking to its own loopback server (relative ./api fetch)', + 'lib/ci-audit/templates/ci-audit-weekly.yml': + 'a workflow template the user copies into their own repo: its fetch of the public gstack repo runs on that repo\'s GitHub Actions runner, not on a machine with gstack installed, and sends no gstack or user state', 'scripts/gate-cost-replay.ts': 'developer-invoked read-only measurement (GSTA-49 T0): git fetch of gstack\'s own origin by SHA (old PR heads and tested merge commits) so past PR gates can be replayed; a download only, no gstack or user state leaves the machine', 'scripts/ubicloud':