diff --git a/.github/workflows/release-gate.yml b/.github/workflows/release-gate.yml index 4f92f95bc..688fb1e72 100644 --- a/.github/workflows/release-gate.yml +++ b/.github/workflows/release-gate.yml @@ -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 diff --git a/docs/ship-policy.md b/docs/ship-policy.md index 3632b1313..3ec2fd63a 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -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); diff --git a/lib/release-gate.ts b/lib/release-gate.ts index bdedc6b05..e424d2c46 100644 --- a/lib/release-gate.ts +++ b/lib/release-gate.ts @@ -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 ?? '-'}`); diff --git a/test/release-gate-workflow.test.ts b/test/release-gate-workflow.test.ts index e881221c9..9f4e5402e 100644 --- a/test/release-gate-workflow.test.ts +++ b/test/release-gate-workflow.test.ts @@ -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 }}' }); }); diff --git a/test/release-gate.test.ts b/test/release-gate.test.ts index cfee00ce7..03c3103d1 100644 --- a/test/release-gate.test.ts +++ b/test/release-gate.test.ts @@ -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', () => {