mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-03 15:27:56 +01:00
fix: pre-landing review fixes, pass 2 (the workflow reads main head itself; an open gate on stable or older than three cadences)
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
cbe0bc81ad
commit
70ba48cb1e
5 files changed
+37
-7
No files matched your search
@@ -85,7 +85,7 @@ jobs:
|
||||
DRY_RUN: ${{ inputs.dry_run && '1' || '' }}
|
||||
IGNORE_CADENCE: ${{ inputs.ignore_cadence && '1' || '' }}
|
||||
run: |
|
||||
./bin/gstack-release-gate run --repo "$GITHUB_REPOSITORY" --sha "$GITHUB_SHA" --budget-minutes 330 \
|
||||
./bin/gstack-release-gate run --repo "$GITHUB_REPOSITORY" --budget-minutes 330 \
|
||||
--summary "$GITHUB_STEP_SUMMARY" --outputs "$GITHUB_OUTPUT" ${DRY_RUN:+--dry-run} ${IGNORE_CADENCE:+--ignore-cadence}
|
||||
|
||||
# The gated commit's check run (release_gate.check_name, main-backstop), the
|
||||
|
||||
+2
-1
@@ -1151,7 +1151,8 @@ policy read, so no paid work starts and nothing is written. With the block it ru
|
||||
|
||||
1. first finishes an open gate on an older commit (a run that outlived its
|
||||
budget, or a red gate's unfinished bisect), so main moving never orphans a
|
||||
verdict; only then does it look at main's head;
|
||||
verdict; only then does it look at main's head. An open gate older than
|
||||
three cadences stops holding up the head;
|
||||
2. skips when main has not moved since `stable` or since the last gate, or
|
||||
when the last gate is younger than `cadence_hours` (`--ignore-cadence`
|
||||
overrides only the age);
|
||||
|
||||
+5
-2
@@ -41,6 +41,8 @@ export const MAX_RELEASE_ATTEMPTS = 2;
|
||||
/** Cron start jitter the cadence forgives. */
|
||||
export const CADENCE_GRACE_HOURS = 1;
|
||||
export const MAX_BISECT_ROUNDS = 4;
|
||||
/** An open gate (a run past its budget, or a red gate's running bisect) on an older commit is finished first for this many cadences; after that it stops holding up main's head. */
|
||||
export const OPEN_GATE_CADENCES = 3;
|
||||
const SHA = /^[0-9a-f]{40}$/;
|
||||
const CASE_ID = /^[A-Za-z0-9 _.:/-]{1,128}$/;
|
||||
const HOUR = 3_600_000;
|
||||
@@ -232,14 +234,15 @@ export function runGate(env: GateEnv, o: RunOptions): GateOut {
|
||||
if ('error' in c) { lines.push(`RELEASE: cannot read refs/notes/gstack-trains (${c.error}); nothing dispatched`); return out('failed', null); }
|
||||
const head = o.sha ?? env.git(['ls-remote', env.remote, `refs/heads/${gate.branch}`]).out.split(/\s/)[0] ?? '';
|
||||
if (!SHA.test(head)) { lines.push(`RELEASE: cannot resolve ${gate.branch} on ${env.remote} (${JSON.stringify(head)})`); return out('failed', null); }
|
||||
const open = o.sha ? null : c.records.find(r => r.sha !== head && ((r.verdict === 'PENDING' && r.run) || (r.verdict === 'RED' && r.bisect?.state === 'running')));
|
||||
const fresh = (r: ReleaseRecord) => env.now() - Date.parse(r.created_at) < OPEN_GATE_CADENCES * gate.cadence_hours * HOUR;
|
||||
const open = o.sha ? null : c.records.find(r => r.sha !== head && fresh(r) && ((r.verdict === 'PENDING' && r.run) || (r.verdict === 'RED' && r.bisect?.state === 'running')));
|
||||
if (open && o.dryRun) { lines.push(`RELEASE: dry run: would finish the open gate on ${short(open.sha)} (${open.verdict}, run ${open.run?.id ?? '-'}) before gating ${gate.branch}@${short(head)}; nothing written`); return out('dry-run', open.sha); }
|
||||
if (open) lines.push(`RELEASE: finishing the open gate on ${short(open.sha)} (${open.verdict}, run ${open.run?.id ?? '-'}) before gating ${gate.branch}@${short(head)}`);
|
||||
const sha = open?.sha ?? head;
|
||||
ensureHistory(env, gate, sha);
|
||||
const prior = c.records.find(r => r.sha === sha) ?? null;
|
||||
const skip = (why: string) => { lines.push(`RELEASE: skip sha=${sha} (${why})`); return out('skipped', sha); };
|
||||
if (sha === c.stable) return skip(`${gate.branch} has not moved since ${gate.stable_ref}`);
|
||||
if (!open && sha === c.stable) return skip(`${gate.branch} has not moved since ${gate.stable_ref}`);
|
||||
if (prior?.verdict === 'GREEN' || prior?.verdict === 'RED') {
|
||||
if (prior.verdict === 'RED' && prior.bisect?.state === 'running' && !o.dryRun) return finishRed(env, c, prior, lines, out);
|
||||
return skip(`${gate.branch} has not moved since the last gate: ${prior.verdict} run ${prior.run?.id ?? '-'}`);
|
||||
|
||||
@@ -55,7 +55,8 @@ const TRAIN = { gate_workflow: 'evals.yml', record_artifact: 'gstack-train-recor
|
||||
|
||||
describe('release-gate.yml', () => {
|
||||
// Value: protects=the schedule fires every RELEASE_GATE_SCHEDULE_HOURS (the cadence floor the policy validates), one gate at a time, only from the
|
||||
// default branch, with contents and actions write but no checks write, and runs the bin on the run's own sha with its outputs wired; the policy step
|
||||
// default branch, with contents and actions write but no checks write, and runs the bin without --sha (main's head from the remote, so an open gate on an older commit
|
||||
// is finished first) with its outputs wired; the policy step
|
||||
// uses git and jq only and is inert without a release_gate block; fails_when=the cron and the cadence drift apart, two gates overlap, the job that
|
||||
// runs repository code can write check runs, or an inert policy reaches bun; why_new=new workflow; seam=none; no_claim=GitHub's cron jitter
|
||||
test('schedule, concurrency, permissions and the gate step', () => {
|
||||
@@ -76,7 +77,8 @@ describe('release-gate.yml', () => {
|
||||
expect(runStep(policy, { release_gate: GATE })).toEqual({ release: 'true' });
|
||||
const gate = job.steps.find(s => s.id === 'gate')!;
|
||||
expect(gate.if).toBe("steps.policy.outputs.release == 'true'");
|
||||
expect(gate.run).toContain('./bin/gstack-release-gate run --repo "$GITHUB_REPOSITORY" --sha "$GITHUB_SHA"');
|
||||
expect(gate.run).toContain('./bin/gstack-release-gate run --repo "$GITHUB_REPOSITORY" --budget-minutes');
|
||||
expect(gate.run).not.toContain('--sha');
|
||||
expect(gate.run).toContain('--outputs "$GITHUB_OUTPUT"');
|
||||
expect(job.outputs).toEqual({ action: '${{ steps.gate.outputs.action }}', sha: '${{ steps.gate.outputs.sha }}', check: '${{ steps.gate.outputs.check }}' });
|
||||
});
|
||||
|
||||
@@ -13,7 +13,7 @@ import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import { bunIsolationArgs } from '../lib/bun-isolation';
|
||||
import {
|
||||
MAX_RELEASE_ATTEMPTS, activeHold, bisectGate, classifyRun, gateStatus, loadReleaseGate, narrow, parseReport, planPrefixes, releaseRecords, runGate,
|
||||
MAX_RELEASE_ATTEMPTS, OPEN_GATE_CADENCES, activeHold, bisectGate, classifyRun, gateStatus, loadReleaseGate, narrow, parseReport, planPrefixes, releaseRecords, runGate,
|
||||
type GateEnv, type Run,
|
||||
} from '../lib/release-gate';
|
||||
import { RELEASE_GATE_SCHEDULE_HOURS, releaseGateErrors, type ReleaseGatePolicy } from '../lib/release-gate-policy';
|
||||
@@ -202,6 +202,30 @@ describe('release gate: run', () => {
|
||||
expect(fake.dispatches).toHaveLength(2);
|
||||
expect(fake.dispatches[1]!.inputs.expected_sha).toBe(next);
|
||||
});
|
||||
|
||||
// Value: protects=an open gate is finished even after release-cut moved stable onto its commit, and an open gate older than OPEN_GATE_CADENCES
|
||||
// cadences stops holding up main's head; fails_when=an open record on stable's commit skips forever, or an unresolvable run blocks every later gate;
|
||||
// why_new=review findings (state-machines and red-team, l1-p2); seam=fake gh; no_claim=a real deleted run
|
||||
test('an open gate on the commit stable moved to is still finished; a stale open gate no longer blocks the head', () => {
|
||||
const f = fixture();
|
||||
let dispatched = 0;
|
||||
const fake = fakeGh(() => (dispatched++ === 0 ? { conclusion: 'success', status: 'in_progress', artifacts: report('GREEN') } : green()));
|
||||
expect(runGate({ ...envFor(f, fake.gh), deadline: 0 }, { gate: GATE, label: 'test', dryRun: false, ignoreCadence: true }).action).toBe('failed');
|
||||
git(f.seed, 'push', '-q', '-f', 'origin', f.c[4] + ':refs/heads/stable');
|
||||
f.commit('c6');
|
||||
fake.runs[0]!.status = 'completed';
|
||||
const finished = runGate(envFor(f, fake.gh), { gate: GATE, label: 'test', dryRun: false, ignoreCadence: true });
|
||||
expect(finished.action, finished.lines.join('\n')).toBe('gated');
|
||||
expect(finished.sha).toBe(f.c[4]);
|
||||
|
||||
const g = fixture();
|
||||
const fake2 = fakeGh(() => ({ conclusion: 'success', status: 'in_progress', artifacts: report('GREEN') }));
|
||||
expect(runGate({ ...envFor(g, fake2.gh), deadline: 0 }, { gate: GATE, label: 'test', dryRun: false, ignoreCadence: true }).action).toBe('failed');
|
||||
const head = g.commit('c6');
|
||||
const late = runGate({ ...envFor(g, fake2.gh, OPEN_GATE_CADENCES * GATE.cadence_hours + 1), deadline: 0 }, { gate: GATE, label: 'test', dryRun: false, ignoreCadence: true });
|
||||
expect(late.lines.some(l => l.includes('finishing the open gate'))).toBe(false);
|
||||
expect(fake2.dispatches.at(-1)!.inputs.expected_sha).toBe(head);
|
||||
});
|
||||
});
|
||||
|
||||
describe('release gate: red, bisect and the hold', () => {
|
||||
|
||||
Reference in new issue
Block a user