From 0ead11c8e20a82e249f36c3abf89599dfe602324 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Wed, 11 Nov 2026 11:18:15 -0800 Subject: [PATCH] Stop the stable cut counting its own check runs against the commit it cuts (GST-2463) (#4297) * Stop the stable cut counting its own check runs against the commit it cuts (GST-2463) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Match the stable cut's own check runs by job, not by check suite (GST-2463) GitHub files a check run another workflow creates through the API (main-backstop from release-gate.yml and evals-main-backstop.yml) under any github-actions check suite on the commit, so suite membership could hide a red main-backstop. The cut now lists the jobs of its own workflow runs and sets aside only check runs whose id is one of those job ids. Review-Finding: p1/red-team#1 reader=red-team Review-Finding: p1/adversarial-native#1 reader=adversarial-native Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Say that the stable cut's own-run lookup needs actions: read and fails safe (GST-2463) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- .github/workflows/weekly-release.yml | 3 + .../stable-cut-ignores-its-own-check-runs.md | 4 + crew/scripts/release-cut.ts | 23 +++++- crew/test/release-cut.test.ts | 79 ++++++++++++++++++- docs/releases.md | 8 +- test/weekly-release-workflow.test.ts | 4 +- 6 files changed, 113 insertions(+), 8 deletions(-) create mode 100644 changelog.d/stable-cut-ignores-its-own-check-runs.md diff --git a/.github/workflows/weekly-release.yml b/.github/workflows/weekly-release.yml index 768dfcf4f..3c9ea8756 100644 --- a/.github/workflows/weekly-release.yml +++ b/.github/workflows/weekly-release.yml @@ -77,6 +77,8 @@ jobs: contents: write # Reads the candidate's check runs. checks: read + # Finds this workflow's own runs on the candidate, whose check runs the gate sets aside. + actions: read # A security or regression cut reads the label on the # its reason names. issues: read pull-requests: read @@ -143,6 +145,7 @@ jobs: permissions: contents: read checks: read + actions: read steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 with: diff --git a/changelog.d/stable-cut-ignores-its-own-check-runs.md b/changelog.d/stable-cut-ignores-its-own-check-runs.md new file mode 100644 index 000000000..c0a6dbf20 --- /dev/null +++ b/changelog.d/stable-cut-ignores-its-own-check-runs.md @@ -0,0 +1,4 @@ +--- +important: blocker +--- +- **The stable release no longer blocks itself on its own check run.** The daily cut runs on main's newest commit, so its own "Cut the release" job showed up as a check run on the very commit it might release. While the cut ran, that commit looked unfinished and was skipped; when a cut failed, its red check run kept that commit out of every later cut. On 2026-11-11 this held v1.99.1.0 back after the release gate had passed it. The cut now sets aside check runs that belong to its own workflow (`.github/workflows/weekly-release.yml`, matched through the GitHub Actions run that produced them, not by job name) and judges every other check run exactly as before: a red or running check from any other workflow, a missing or running `free-tests`, a missing gate, a RED release record and a freeze still refuse. The workflow now also needs `actions: read` to look up its own runs; if that lookup fails, nothing is set aside and the cut behaves as it did before. diff --git a/crew/scripts/release-cut.ts b/crew/scripts/release-cut.ts index 8a10caf62..167b7b3fa 100755 --- a/crew/scripts/release-cut.ts +++ b/crew/scripts/release-cut.ts @@ -19,7 +19,13 @@ * failed, and the trains notes ref (--notes-ref, default * refs/notes/gstack-trains) holds no active freeze. The newest such commit in * the window (7 days, 200 first-parent commits) is the candidate; a commit - * with a check run still going is skipped, not waited for. + * with a check run still going is skipped, not waited for. Check runs this + * workflow produced itself (CUT_WORKFLOW: the jobs of a workflow run whose path + * is that file; a job's id is its check run's id) are set aside, running or + * finished: a cut lands its own job on main's head, the commit it may cut. + * Matching by job, not check suite, keeps check runs other workflows create + * through the API, which GitHub files under any github-actions suite on the + * commit. When the workflow runs cannot be read, nothing is set aside. * * Gated means one of three kinds of evidence. Either the backstop check run * (--backstop-check, default main-backstop) succeeded: evals-main-backstop.yml @@ -70,6 +76,7 @@ export type Git = (args: string[]) => { ok: boolean; out: string }; export const REQUIRED_CHECK = 'free-tests'; export const BACKSTOP_CHECK = 'main-backstop'; export const NOTES_REF = 'refs/notes/gstack-trains'; +export const CUT_WORKFLOW = '.github/workflows/weekly-release.yml'; const NOTES_LOCAL = 'refs/notes/gstack-trains-release'; const RED = new Set(['failure', 'timed_out', 'cancelled', 'action_required', 'startup_failure', 'stale']); export const WINDOW_DAYS = 7; @@ -103,7 +110,7 @@ export function compareVersions(a: string, b: string): number { /** Green names what gated the commit: the backstop check's name, or `train `. Pending means a check run on it is still going. */ export type Verdict = { green: true; via: string } | { green: false; why: string; pending: boolean }; -interface CheckRun { name: string; status: string; conclusion: string | null } +interface CheckRun { id?: number; name: string; status: string; conclusion: string | null } /** Main commits a green, merging or merged train record lists in merged[], by sha and by tree, each naming the train; `release` maps a commit a green release gate passed to its run (crew/lib/release-gate.ts). */ export interface TrainEvidence { bySha: Map; byTree: Map; release?: Map } @@ -116,7 +123,17 @@ export function gateVerdict(gh: Gh, repo: string, sha: string, o: GateOptions = const paged = parseJson(gh(['api', '--paginate', '--slurp', `repos/${repo}/commits/${sha}/check-runs?per_page=100`])); if (!paged) return { green: false, why: 'cannot read its check runs', pending: false }; const pages = (Array.isArray(paged) ? paged : [paged]) as Array<{ check_runs?: CheckRun[] }>; - const runs = pages.flatMap(p => p.check_runs ?? []); + let runs = pages.flatMap(p => p.check_runs ?? []); + if (runs.some(c => c.status !== 'completed' || RED.has(c.conclusion ?? ''))) { + const wf = parseJson(gh(['api', '--paginate', '--slurp', `repos/${repo}/actions/runs?head_sha=${sha}&per_page=100`])); + const wfRuns = (Array.isArray(wf) ? wf : wf ? [wf] : []) as Array<{ workflow_runs?: Array<{ id?: number; path?: string }> }>; + const own = new Set(); + for (const w of wfRuns.flatMap(p => p.workflow_runs ?? []).filter(w => w.path === CUT_WORKFLOW && typeof w.id === 'number')) { + const jobs = parseJson(gh(['api', '--paginate', '--slurp', `repos/${repo}/actions/runs/${w.id}/jobs?filter=all&per_page=100`])); + for (const p of (Array.isArray(jobs) ? jobs : jobs ? [jobs] : []) as Array<{ jobs?: Array<{ id?: number }> }>) for (const j of p.jobs ?? []) if (typeof j.id === 'number') own.add(j.id); + } + runs = runs.filter(c => !(typeof c.id === 'number' && own.has(c.id))); + } let via = backstop; for (const name of [required, backstop]) { const named = runs.filter(c => c.name === name); diff --git a/crew/test/release-cut.test.ts b/crew/test/release-cut.test.ts index 5ca1c5fb8..7abf14709 100644 --- a/crew/test/release-cut.test.ts +++ b/crew/test/release-cut.test.ts @@ -13,8 +13,8 @@ import * as path from 'node:path'; import { bunIsolationArgs } from '../../lib/bun-isolation'; import { gitFixtureTemplate, rebindGitDir } from '../../test/helpers/git-fixture-template'; import { - BACKSTOP_CHECK, FLOOR_HOURS, NOTES_CAP, NOTES_REF, REQUIRED_CHECK, SCHEDULED_GRACE_HOURS, changelogEntries, compareVersions, cutRelease, describePlan, floorStatus, - gateVerdict, gitIn, planRelease, readTrainNotes, releaseNotes, sanitizeReason, stableMovedAt, verifyRegression, verifySecurity, type Gh, type Git, + BACKSTOP_CHECK, CUT_WORKFLOW, FLOOR_HOURS, NOTES_CAP, NOTES_REF, REQUIRED_CHECK, SCHEDULED_GRACE_HOURS, changelogEntries, compareVersions, cutRelease, describePlan, floorStatus, + gateVerdict, gitIn, planRelease, readTrainNotes, releaseNotes, sanitizeReason, stableMovedAt, verifyRegression, verifySecurity, type Gh, type Git, type TrainEvidence, } from '../scripts/release-cut'; const bases: string[] = []; @@ -306,6 +306,81 @@ describe('release-cut: the release gate holds and gates (crew/lib/release-gate.t }); }); +describe('release-cut: the cut sets aside its own check runs (GST-2463)', () => { + const OWN_JOB = 70001; + const ownRun = (status: string, conclusion: string | null, id = OWN_JOB) => ({ id, name: 'Cut the release', status, conclusion }) as CheckRun; + const workflowRuns = (sha: string, runs: Array<{ id: number; path: string; jobs: number[] }>) => Object.assign( + { ['repos/o/r/actions/runs?head_sha=' + sha + '&per_page=100']: [{ total_count: runs.length, workflow_runs: runs.map(({ id, path }) => ({ id, path, check_suite_id: id })) }] }, + ...runs.map(r => ({ ['repos/o/r/actions/runs/' + r.id + '/jobs?filter=all&per_page=100']: [{ total_count: r.jobs.length, jobs: r.jobs.map(id => ({ id })) }] }))); + const ownWorkflow = { id: 9001, path: CUT_WORKFLOW, jobs: [OWN_JOB] }; + const unbacked = GREEN.filter(c => c.name !== BACKSTOP_CHECK); + const gatedBy = (sha: string, run: string): TrainEvidence => ({ bySha: new Map(), byTree: new Map(), release: new Map([[sha, run]]) }); + const sha = 'c'.repeat(40); + const ghFor = (checks: CheckRun[], api: Record = workflowRuns(sha, [ownWorkflow, { id: 555, path: '.github/workflows/evals.yml', jobs: [70555] }])) => + fakeGh({ checks: { [sha]: checks }, released: new Set(), calls: [], api }); + + // Value: protects=the stable cut's own check run (in progress while it runs, red once it fails) never keeps a commit gated by a GREEN release record out + // of the cut, while a red or running check run from any other workflow, a running required check, a missing gate, a same-named job of another + // workflow and a check run another workflow created through the API (which GitHub may file under the cut's check suite) still do, and an unreadable + // workflow-runs API sets nothing aside; fails_when=the cut skips main's head because its own job is still running, a failed cut poisons the commit, + // or the exclusion is keyed on the display name or the check suite; why_new=gateVerdict judged every check run on the commit, including the cut's + // own (run 43076123863 on 19ed2ac5); seam=gh is faked with the check-runs, actions/runs and jobs payload shapes; no_claim=a live workflow run + test('gateVerdict: the cut\'s own running or failed check run is set aside; every other not-green signal still refuses', () => { + const evidence = gatedBy(sha, '43072520821'); + for (const own of [ownRun('in_progress', null), ownRun('queued', null), ownRun('completed', 'failure')]) { + expect(gateVerdict(ghFor([...unbacked, own]), 'o/r', sha, { evidence })).toEqual({ green: true, via: 'release gate run 43072520821' }); + } + expect(gateVerdict(ghFor([...unbacked, ownRun('completed', 'failure'), { name: 'quality', status: 'completed', conclusion: 'failure' }]), 'o/r', sha, { evidence })) + .toEqual({ green: false, why: 'red check runs: quality', pending: false }); + expect(gateVerdict(ghFor([...unbacked, ownRun('completed', 'failure', 70555)]), 'o/r', sha, { evidence })) + .toEqual({ green: false, why: 'red check runs: Cut the release', pending: false }); + expect(gateVerdict(ghFor([...unbacked, ownRun('in_progress', null, 80002)]), 'o/r', sha, { evidence })) + .toEqual({ green: false, why: 'check runs still going: Cut the release', pending: true }); + expect(gateVerdict(ghFor([...unbacked, ownRun('completed', 'failure'), { id: 80001, name: BACKSTOP_CHECK, status: 'completed', conclusion: 'failure', app: { slug: 'github-actions' }, check_suite: { id: 9001 } } as CheckRun]), 'o/r', sha, { evidence })) + .toEqual({ green: false, why: 'red check runs: ' + BACKSTOP_CHECK, pending: false }); + expect(gateVerdict(ghFor([{ name: REQUIRED_CHECK, status: 'in_progress', conclusion: null }, ownRun('in_progress', null)]), 'o/r', sha, { evidence })) + .toEqual({ green: false, why: REQUIRED_CHECK + ' is still running', pending: true }); + expect(gateVerdict(ghFor([...unbacked, ownRun('completed', 'failure')]), 'o/r', sha)) + .toEqual({ green: false, why: 'no ' + BACKSTOP_CHECK + ' check run', pending: false }); + expect(gateVerdict(ghFor([...unbacked, ownRun('completed', 'failure')], {}), 'o/r', sha, { evidence })) + .toEqual({ green: false, why: 'red check runs: Cut the release', pending: false }); + }); + + const note = (f: ReturnType, record: unknown, on: string) => { + git(f.seed, 'notes', '--ref=' + NOTES_REF, 'add', '-f', '-m', JSON.stringify(record), on); + git(f.seed, 'push', '-q', '-f', 'origin', NOTES_REF); + }; + const releaseRecord = (on: string, verdict: string, run: number) => ({ schema: 1, kind: 'release', repo_key: 'k', sha: on, verdict, run: { id: run, url: 'u' }, red_cases: verdict === 'RED' ? ['case-a'] : [], created_at: 'x', updated_at: 'x' }); + + // Value: protects=the 11-11 deadlock: with main's head still running its backstop and the cut's own job, a release-gated commit carrying a failed cut's + // red check run, and stable on the older released commit, the cut picks the gated commit instead of falling back to the released one; a RED release + // record still refuses; fails_when=the cut skips the gated commit for its own red check run and reports NO_RELEASE_PR or NOTHING_NEW, or the + // exclusion lifts the release-gate hold; why_new=no test put the cut's own check runs on a candidate; seam=gh faked, git real; no_claim=a live cut + test('the 11-11 state: head running its backstop and the cut, a gated commit with the failed cut\'s red run, stable on the released commit before it', () => { + const f = fixture(); + const released = f.shas['1.0.2']!, gated = f.shas['1.0.3']!; + execFileSync('git', ['tag', '-a', 'v1.0.2', released, '-m', 'gstack v1.0.2'], { cwd: f.seed, timeout: 30_000, env: { ...process.env, GIT_COMMITTER_NAME: 't', GIT_COMMITTER_EMAIL: 't@x' } }); + git(f.seed, 'push', '-q', 'origin', 'refs/tags/v1.0.2', released + ':refs/heads/stable'); + f.state.released.add('v1.0.2'); + const head = f.release('1.0.4', 'Fifth is still being gated.'); + f.refresh(); + note(f, releaseRecord(gated, 'GREEN', 43072520821), gated); + f.state.checks[head] = [...backstop('in_progress', null), ownRun('in_progress', null, 70002)]; + f.state.checks[gated] = [...unbacked, ownRun('completed', 'failure')]; + f.state.api = { ...workflowRuns(head, [{ id: 9002, path: CUT_WORKFLOW, jobs: [70002] }]), ...workflowRuns(gated, [ownWorkflow]) }; + const r = planRelease({ repo: f.repo, git: f.git, gh: f.gh, now: pastFloor }); + expect(r.ok, JSON.stringify(r)).toBe(true); + if (!r.ok) return; + expect(r.plan.sha).toBe(gated); + expect(r.plan.via).toBe('release gate run 43072520821'); + expect(r.plan.skipped).toEqual([head.slice(0, 12) + ': ' + BACKSTOP_CHECK + ' is still running']); + note(f, releaseRecord(gated, 'RED', 43072520822), gated); + const held = planRelease({ repo: f.repo, git: f.git, gh: f.gh, now: pastFloor }); + expect(held.ok).toBe(false); + if (!held.ok) { expect(held.code).toBe('REFUSED'); expect(held.message).toContain('release gate red on ' + gated.slice(0, 12)); } + }); +}); + describe('release-cut: train evidence gates a commit whose backstop never settled', () => { const train = (nonce: string, state: string, merged: Array<{ sha: string; tree: string }>) => ({ schema: 1, kind: 'train', nonce, state, merged: merged.map(m => ({ pr: '1', at: 'x', ...m })), updated_at: 'x' }); diff --git a/docs/releases.md b/docs/releases.md index 6e1bf74f1..43506b18b 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -46,7 +46,13 @@ regression fix (below). it has completed and none failed, and `refs/notes/gstack-trains` holds no active freeze. A cancelled check run counts as red. A commit with a check run still going is skipped and the next older one is considered, back through 7 -days or 200 first-parent commits. The workflow runs no tests and no paid evals +days or 200 first-parent commits. The cut's own job lands a check run on +main's head, the very commit it may cut, so check runs from this workflow's +own runs (its file path, `.github/workflows/weekly-release.yml`, matched +by the job ids of those workflow runs) are set aside whether running or +failed; every other check run counts as above. The lookup needs the +workflow's `actions: read` permission; when it cannot read those runs, nothing +is set aside. The workflow runs no tests and no paid evals of its own; it only reads what the trains, the backstop and the release gate already recorded. With nothing green in the window it fails and lists why each commit was diff --git a/test/weekly-release-workflow.test.ts b/test/weekly-release-workflow.test.ts index c5d6b17b2..0ad32aaef 100644 --- a/test/weekly-release-workflow.test.ts +++ b/test/weekly-release-workflow.test.ts @@ -49,7 +49,7 @@ describe('weekly-release.yml', () => { test('the cut job runs only off main for a schedule or dispatch, writes contents, reads checks, and rejects a ref that is not a 40-hex SHA', () => { const cut = wf.jobs.cut!; expect(cut.if).toBe("github.event_name != 'pull_request' && github.ref == 'refs/heads/main'"); - expect(cut.permissions).toEqual({ contents: 'write', checks: 'read', issues: 'read', 'pull-requests': 'read' }); + expect(cut.permissions).toEqual({ contents: 'write', checks: 'read', actions: 'read', issues: 'read', 'pull-requests': 'read' }); expect(cut.steps.find(s => s.uses?.startsWith('actions/checkout@'))!.with).toEqual({ 'fetch-depth': 0 }); const plan = cut.steps.find(s => s.name === 'Plan and cut')!; expect(plan.env!.GH_TOKEN).toBe('${{ github.token }}'); @@ -78,7 +78,7 @@ describe('weekly-release.yml', () => { test('the pull-request dry run is read-only, keeps no credentials, and always passes --dry-run', () => { const dry = wf.jobs['dry-run']!; expect(dry.if).toBe("github.event_name == 'pull_request'"); - expect(dry.permissions).toEqual({ contents: 'read', checks: 'read' }); + expect(dry.permissions).toEqual({ contents: 'read', checks: 'read', actions: 'read' }); expect(dry.steps.find(s => s.uses?.startsWith('actions/checkout@'))!.with).toEqual({ 'fetch-depth': 0, 'persist-credentials': false }); const plan = dry.steps.find(s => s.name === 'Plan only')!; expect(plan.run).toContain(BUN_RUN + ' --repo "$GITHUB_REPOSITORY" --summary "$GITHUB_STEP_SUMMARY" --dry-run\n');