mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-03 15:27:56 +01:00
fix: pre-landing review fixes (finish an open gate before a new head; neutral check for an unfinished gate)
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
080bc1c156
commit
cbe0bc81ad
6 files changed
+57
-16
No files matched your search
@@ -108,7 +108,8 @@ jobs:
|
||||
run: |
|
||||
case "$ACTION" in
|
||||
gated) conclusion=success ;;
|
||||
held|failed) conclusion=failure ;;
|
||||
held) conclusion=failure ;;
|
||||
failed) conclusion=neutral ;;
|
||||
*) echo "::error::unknown release gate action '$ACTION'"; exit 1 ;;
|
||||
esac
|
||||
[[ "$SHA" =~ ^[0-9a-f]{40}$ ]] || { echo "::error::'$SHA' is not a commit SHA"; exit 1; }
|
||||
|
||||
+12
-7
@@ -1149,22 +1149,27 @@ on a manual dispatch from main. Without the block it stops after a git-and-jq
|
||||
policy read, so no paid work starts and nothing is written. With the block it runs
|
||||
`gstack-release-gate run`, which:
|
||||
|
||||
1. skips when main has not moved since `stable` or since the last gate, or
|
||||
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;
|
||||
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);
|
||||
2. writes a `release` record (`gstack-trains:release:<repo>:<sha>`) to
|
||||
3. writes a `release` record (`gstack-trains:release:<repo>:<sha>`) to
|
||||
`refs/notes/gstack-trains` before it dispatches, so a lost run resumes;
|
||||
3. dispatches the full gate on that sha and reads its verdict and flake
|
||||
4. dispatches the full gate on that sha and reads its verdict and flake
|
||||
records. A case that failed once and passed its one rule-16 rerun is green
|
||||
and is listed as flaky;
|
||||
4. records GREEN, RED or INCOMPLETE. An INCOMPLETE gate is re-dispatched once
|
||||
5. records GREEN, RED or INCOMPLETE. An INCOMPLETE gate is re-dispatched once
|
||||
by the next run, then left for a person;
|
||||
5. on RED, bisects the red cases over main's first-parent commits since the
|
||||
6. on RED, bisects the red cases over main's first-parent commits since the
|
||||
last green gate (at most 4 rounds, within the spend cap) and records the
|
||||
first bad commit;
|
||||
6. publishes `check_name` on the gated commit from a separate job that holds
|
||||
7. publishes `check_name` on the gated commit from a separate job that holds
|
||||
`checks: write` and runs no repository code: success for a green gate,
|
||||
failure for a red or failed one.
|
||||
failure for a red one, and neutral when the gate has no verdict yet (still
|
||||
running at the budget, or incomplete), so a later green on the same commit
|
||||
is not undercut by an earlier failure.
|
||||
|
||||
**What release-cut reads.** A GREEN release record on a commit counts as
|
||||
gated, the same as a successful `main-backstop` check run. A RED record
|
||||
|
||||
+6
-2
@@ -230,8 +230,12 @@ export function runGate(env: GateEnv, o: RunOptions): GateOut {
|
||||
const gate = o.gate;
|
||||
const c = context(env, gate);
|
||||
if ('error' in c) { lines.push(`RELEASE: cannot read refs/notes/gstack-trains (${c.error}); nothing dispatched`); return out('failed', null); }
|
||||
const sha = o.sha ?? env.git(['ls-remote', env.remote, `refs/heads/${gate.branch}`]).out.split(/\s/)[0] ?? '';
|
||||
if (!SHA.test(sha)) { lines.push(`RELEASE: cannot resolve ${gate.branch} on ${env.remote} (${JSON.stringify(sha)})`); 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')));
|
||||
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); };
|
||||
|
||||
@@ -17,7 +17,7 @@
|
||||
"scripts/lib/published-text.ts": {"blob":"3f2e4d616b5dd0204035710c66b64d49055c5714","since":6},
|
||||
"scripts/lib/shard-engine.ts": {"blob":"dc714d7d897aed185c0ec3d2c92ad22d954039e5","since":6,"nonBehavioral":"createShardSandbox drops the Capy launch markers (GSTACK_SESSION_KIND, GSTACK_CAPY, GSTACK_EPHEMERAL, GSTACK_INSTALL_VERDICT) from shard environments; CI never sets them, so no case's input or verdict changes there, and on a Capy machine fixtures now see the session kind they set themselves instead of the installer's"},
|
||||
"scripts/test-paid-shards.ts": {"blob":"5439d0a8f6e1569fa7c924af20d67c30c81d5d30","since":6,"nonBehavioral":"credential rejections are classed infra instead of assertion: no case's pass or fail changes; only failed trials whose provider rejected the run's key move class, so no rerun spends on a rejected key"},
|
||||
"scripts/test-pr-profile.ts": {"blob":"dd0ca92bcf8493fd5c1e9d399cc3bc74ec70c469","since":6,"nonBehavioral":"FREE_ONLY_PR_FILES gains USING_GBRAIN_WITH_GSTACK.md (a root guide no skill prompt or paid fixture reads); it changes only which PR paths select paid cases, never how a case runs or is judged (GST-1027)"},
|
||||
"scripts/test-pr-profile.ts": {"blob":"401bda3a3243bc836f39ac4dd0e5933ddcbaee17","since":6,"nonBehavioral":"scripts/test-pr-profile.ts only classifies the new release-gate.yml as free-only; no case selection or verdict reads that entry"},
|
||||
"scripts/test-strict-output.ts": {"blob":"01185d20dc9761b04b9b2f751ec98664fe0eaf93","since":6},
|
||||
"test/helpers/agent-sdk-runner.ts": {"blob":"d66dac458b8c13250973e550edf8087d4044cd0b","since":6,"nonBehavioral":"Cost attribution only (GSTA-150 PR 2): a display-label record carries cost_case_id from runSkillTest's testName and the report binds its cost by it while the failed record and failure class still come from the named records (test/paid-report-cost.test.ts pins both); an aborted Agent SDK session logs its streamed usage as streamed_cost_usd, used only when a case has no record; a listed model-free case with no record or session is a known $0. No case's execution, outcome, failure class, rerun eligibility or panel verdict changes, so no pass-rate series resets. Also: runAgentSdkTest now validates systemPrompt before starting; every live caller already passed the claude_code preset, so the prompt the model sees is unchanged and no verdict can change"},
|
||||
"test/helpers/auq-substance-panel.ts": {"blob":"4881260ef793668e4963a062db6d06a5e1f7a40f","since":6},
|
||||
|
||||
@@ -82,8 +82,9 @@ describe('release-gate.yml', () => {
|
||||
});
|
||||
|
||||
// Value: protects=the gated commit's check run (what release-cut reads) comes from a job with checks: write and no checkout or action, only for a
|
||||
// gate that ran (success for gated, failure for held or failed), on a validated sha and check name; fails_when=repository code runs with checks:
|
||||
// write, a skipped or inert run publishes a check, or a red gate publishes success; why_new=new workflow; seam=none; no_claim=GitHub's check API
|
||||
// gate that ran (success for gated, failure for held, neutral for failed so a later green on the same sha stays releasable), on a validated sha
|
||||
// and check name; fails_when=repository code runs with checks: write, a skipped or inert run publishes a check, a red gate publishes success, or
|
||||
// an unfinished gate leaves a failure release-cut reads as red; why_new=new workflow; seam=none; no_claim=GitHub's check API
|
||||
test('the check run is published from a code-free job, success only for gated', () => {
|
||||
const job = release.jobs['release-check']!;
|
||||
expect(job.needs).toBe('release-gate');
|
||||
@@ -92,7 +93,8 @@ describe('release-gate.yml', () => {
|
||||
expect(job.steps.filter(s => s.uses)).toEqual([]);
|
||||
const run = job.steps[0]!.run!;
|
||||
expect(run).toContain('gated) conclusion=success ;;');
|
||||
expect(run).toContain('held|failed) conclusion=failure ;;');
|
||||
expect(run).toContain('held) conclusion=failure ;;');
|
||||
expect(run).toContain('failed) conclusion=neutral ;;');
|
||||
expect(run).toContain('[[ "$SHA" =~ ^[0-9a-f]{40}$ ]]');
|
||||
expect(run).toContain('-f name="$CHECK" -f head_sha="$SHA"');
|
||||
expect(run).not.toContain('bun');
|
||||
|
||||
@@ -51,7 +51,7 @@ function fixture() {
|
||||
return { root, origin, seed, work, base, c, commit };
|
||||
}
|
||||
|
||||
interface Outcome { conclusion: string; artifacts: Record<string, Record<string, string>> }
|
||||
interface Outcome { conclusion: string; artifacts: Record<string, Record<string, string>>; status?: string }
|
||||
/** A fake gh: each dispatch becomes a completed run whose outcome the test decides; run list, run view and artifact download read it back. */
|
||||
function fakeGh(onDispatch: (inputs: Record<string, string>, ref: string) => Outcome) {
|
||||
const runs: Run[] = [];
|
||||
@@ -65,7 +65,7 @@ function fakeGh(onDispatch: (inputs: Record<string, string>, ref: string) => Out
|
||||
dispatches.push({ ref, inputs });
|
||||
const id = 1000 + runs.length;
|
||||
const o = onDispatch(inputs, ref);
|
||||
runs.push({ id, status: 'completed', conclusion: o.conclusion, head_sha: inputs.expected_sha!, display_title: 'E2E Evals [eval-bg ' + inputs.nonce + ']', html_url: 'https://github.com/o/r/actions/runs/' + id, run_attempt: 1 });
|
||||
runs.push({ id, status: o.status ?? 'completed', conclusion: o.conclusion, head_sha: inputs.expected_sha!, display_title: 'E2E Evals [eval-bg ' + inputs.nonce + ']', html_url: 'https://github.com/o/r/actions/runs/' + id, run_attempt: 1 });
|
||||
artifacts.set(id, o.artifacts);
|
||||
return ok('');
|
||||
}
|
||||
@@ -173,6 +173,35 @@ describe('release gate: run', () => {
|
||||
expect(fake.dispatches).toHaveLength(MAX_RELEASE_ATTEMPTS);
|
||||
expect(activeHold(records(f), () => true)).toBeNull();
|
||||
});
|
||||
|
||||
// Value: protects=a gate run still going at the budget stays PENDING with its run, and the next run finishes that verdict on the old commit before
|
||||
// gating main's new head, so a red verdict still records RED and holds release-cut after main moved; the new head is gated on the run after;
|
||||
// fails_when=main moving orphans a PENDING record and its red verdict never becomes a hold; why_new=review finding (red-team, l1-p1); seam=fake gh;
|
||||
// no_claim=a real six-hour run
|
||||
test('an unfinished gate is finished on its own commit after main moves, then the new head is gated', () => {
|
||||
const f = fixture();
|
||||
let dispatched = 0;
|
||||
const fake = fakeGh(() => (dispatched++ === 0 ? { conclusion: 'failure', status: 'in_progress', artifacts: report('RED', ['case-a rule FAIL VERDICT']) } : green()));
|
||||
const gate = { ...GATE, bisect_spend_cap_usd: 0 };
|
||||
const first = runGate({ ...envFor(f, fake.gh), deadline: 0 }, { gate, label: 'test', dryRun: false, ignoreCadence: true });
|
||||
expect(first.action).toBe('failed');
|
||||
expect(records(f)[0]).toMatchObject({ sha: f.c[4], verdict: 'PENDING', run: { id: 1000 } });
|
||||
const next = f.commit('c6');
|
||||
fake.runs[0]!.status = 'completed';
|
||||
const dry = runGate(envFor(f, fake.gh), { gate, label: 'test', dryRun: true, ignoreCadence: true });
|
||||
expect(dry.lines[0]).toContain('would finish the open gate on ' + f.c[4]!.slice(0, 12));
|
||||
const second = runGate(envFor(f, fake.gh), { gate, label: 'test', dryRun: false, ignoreCadence: true });
|
||||
expect(second.action, second.lines.join('\n')).toBe('held');
|
||||
expect(second.sha).toBe(f.c[4]);
|
||||
expect(second.lines[0]).toContain('finishing the open gate on ' + f.c[4]!.slice(0, 12));
|
||||
expect(fake.dispatches).toHaveLength(1);
|
||||
expect(activeHold(records(f), () => true)?.sha).toBe(f.c[4]);
|
||||
const third = runGate(envFor(f, fake.gh), { gate, label: 'test', dryRun: false, ignoreCadence: true });
|
||||
expect(third.action, third.lines.join('\n')).toBe('gated');
|
||||
expect(third.sha).toBe(next);
|
||||
expect(fake.dispatches).toHaveLength(2);
|
||||
expect(fake.dispatches[1]!.inputs.expected_sha).toBe(next);
|
||||
});
|
||||
});
|
||||
|
||||
describe('release gate: red, bisect and the hold', () => {
|
||||
|
||||
Reference in new issue
Block a user