ci-audit: review fixes (weekly template fetch and pins, rate headroom, escaped names, fork runs)

- Template fetches GSTACK_REF with git fetch, so a full commit sha works as the
  comment says; actions are pinned to the SHAs gstack's own workflows use; the
  exit code reaches Publish through env and the report is no longer echoed to
  the log.
- The rate-limit preflight keeps a tenth of the hour's limit spare (at most
  500), so a GITHUB_TOKEN with a little of its hour spent still runs.
- Job, runner and workflow names are escaped in the report and issue body.
- ship-check drops runs whose head repository is not the PR's (fork branches
  that share a name).
- Docs say the /ship step lands in a follow-up PR; egress scanner exempts the
  template with its reason.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
garrytanandcapy-ai[bot] committed 2026-10-31 19:00:26 +00:00
1 parent 080268fb43
commit 7bb277840b
8 files changed
+48 -19

No files matched your search

+2 -1
View File
@@ -68,7 +68,8 @@ are unknown. `--record <dir>` 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
+1 -1
View File
@@ -5695,7 +5695,7 @@ Every error is one line, `CI_AUDIT: unavailable <detail> (<code>)`, followed by
<a id="ci-audit-rate-limit"></a>
### `CI_AUDIT: unavailable <n> of <limit> 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.
<a id="ci-audit-call-budget"></a>
### `CI_AUDIT: unavailable call budget of <n> spent (ci_audit_call_budget)`
+2 -2
View File
@@ -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}`);
+7 -4
View File
@@ -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 ? `<br>${l.evidence.slice(0, 3).join('<br>')}` : ''} |`);
for (const l of r.lessons) out.push(`| ${l.id} | ${l.name} | ${STATE[l.state]} | ${esc(l.summary)}${l.evidence.length ? `<br>${l.evidence.slice(0, 3).map(esc).join('<br>')}` : ''} |`);
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';
}
+3 -1
View File
@@ -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` };
+13 -8
View File
@@ -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
+18 -2
View File
@@ -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 <b>', 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 \\<b\\> | 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<string, string> }).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');
});
});
+2
View File
@@ -169,6 +169,8 @@ const SCANNER_EXEMPT: Record<string, string> = {
'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':