mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-13 11:57:19 +01:00
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>
This commit is contained in:
1 parent
19ed2ac5bb
commit
0ead11c8e2
6 files changed
+113
-8
No files matched your search
@@ -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 #<n> 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:
|
||||
|
||||
@@ -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.
|
||||
@@ -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 <nonce>`. 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<string, string>; byTree: Map<string, string>; release?: Map<string, string> }
|
||||
@@ -116,7 +123,17 @@ export function gateVerdict(gh: Gh, repo: string, sha: string, o: GateOptions =
|
||||
const paged = parseJson<unknown>(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<unknown>(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<number>();
|
||||
for (const w of wfRuns.flatMap(p => p.workflow_runs ?? []).filter(w => w.path === CUT_WORKFLOW && typeof w.id === 'number')) {
|
||||
const jobs = parseJson<unknown>(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);
|
||||
|
||||
@@ -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<string, unknown> = 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<typeof fixture>, 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' });
|
||||
|
||||
+7
-1
@@ -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
|
||||
|
||||
@@ -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');
|
||||
|
||||
Reference in new issue
Block a user