mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-10 18:50:18 +01:00
Warn when a branch-only run hides a red PR check, and make ready refuse when its note or base is missing (GST-2205) (#4270)
* Warn when a red check is replaced by a run from another suite, and refuse a plain ready whose note never reached origin (GST-2205) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Fetch the base before ready picks the train or plain path (GST-2205) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Let the train path report its own fetch failure when ready cannot refresh the base (GST-2205) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Give the no-origin/main routing test a complete pregate record (GST-2205) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Give the unpushed ready-note test a complete pregate record (GST-2205) 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
90fded4dd0
commit
4685fa9233
12 files changed
+226
-32
No files matched your search
@@ -37,7 +37,7 @@ import { EXIT, exitTableHelp, formatResult } from '../lib/headless-artifacts';
|
||||
import { versionAtRef } from '../lib/restamp';
|
||||
import { RESULT_CODES } from '../lib/result-codes';
|
||||
import { resolveBaseBranch } from '../lib/ship-policy';
|
||||
import { loadBasePolicy, ready, stamp, verify, type QueueContext, type StepOutput } from '../lib/ship-queue';
|
||||
import { loadBasePolicy, ready, refreshBase, stamp, verify, type QueueContext, type StepOutput } from '../lib/ship-queue';
|
||||
import { attemptFile, listAttempts, prepareGateAhead, recordGateAhead, type QueueFailure } from '../crew/lib/ship-queue-gate';
|
||||
import { logShipCall } from '../lib/ship-call-log';
|
||||
|
||||
@@ -197,8 +197,10 @@ async function main(argv: string[]): Promise<number> {
|
||||
if (gateExit !== undefined && !/^-?\d+$/.test(gateExit)) return usage('--gate-exit needs an exit code');
|
||||
const gate = { gateExit: gateExit === undefined ? undefined : Number(gateExit), gateLog: flag(args, '--gate-log') };
|
||||
if (cmd === 'ready') {
|
||||
const unfetched = refreshBase(ctx);
|
||||
const loaded = loadBasePolicy(ctx);
|
||||
if (!('message' in loaded) && loaded.policy.train) return (await import('../crew/lib/ship-queue-train-cli')).readyMain(ctx, args, { pr, pregate: flag(args, '--pregate'), ...gate });
|
||||
if (unfetched) return finish('ready', { ok: false, lines: [], run: 'none', failure: unfetched });
|
||||
if (['--screen', '--no-screen', '--broad'].some(f => args.includes(f))) process.stderr.write('gstack-ship-queue ready: --screen/--no-screen/--broad ignored: the base policy declares no train block\n');
|
||||
return finish('ready', ready(ctx, { pr, pregate: flag(args, '--pregate'), ...gate }));
|
||||
}
|
||||
|
||||
@@ -0,0 +1,6 @@
|
||||
---
|
||||
important: blocker
|
||||
---
|
||||
- **`gstack-ship-queue ready` and `land` now say when a red check counted as green only because a run from another check suite replaced it.** A lane that dispatches `free-tests.yml` on its branch after a red `pull_request` run gets a green verdict from the branch-only run, which never tested the merge with main. Both commands now print `CI_SUPERSEDED: <check> red run <id> replaced by <id> (suite <a> → <b>)` for each such run. It is a warning only and changes no verdict; a rerun in the same suite prints nothing (GST-2205).
|
||||
- **Without a train policy, `gstack-ship-queue ready` no longer reports READY when its note never reached origin.** Under `changelog_fragments` it exited 0 even when the READY note push failed, so `land --pin` then refused `QUEUE_NOT_READY` because origin had no note. It now refuses with `QUEUE_READY_NOTE_UNPUSHED`, as the train path does, and rerunning the same `ready` command once the push works records it (GST-2205).
|
||||
- **`gstack-ship-queue ready` now fetches the base before it picks the train or plain path.** It read the base policy first and fetched only inside plain `ready`, so a clone with no `origin/main` ref, or a stale one, ran plain `ready` under a train policy and skipped the train-only screen, conflict and READY-receipt steps. It now fetches first, and refuses `QUEUE_BASE_UNFETCHED` when it can't (GST-2205).
|
||||
@@ -29,7 +29,7 @@ import { recordVerdict, screenVerdict, verdictLines, type ScreenOutcome } from '
|
||||
import { execFor, prInfo, repoSlug } from './ship-queue-train-github';
|
||||
import { bindTrainReceipt } from './ship-queue-train-gate';
|
||||
import { extractReceipt } from '../../lib/ship-receipt';
|
||||
import { loadReady, nowIso, persist, readyPath, rebuildFromNotes, type ReadyReceipt, type ScreenChoice } from './ship-queue-train-record';
|
||||
import { loadReady, nowIso, persist, readyNoteUnpushed, readyPath, rebuildFromNotes, type ReadyReceipt, type ScreenChoice } from './ship-queue-train-record';
|
||||
import { baseOverlap, fetchBase, mergeCommit, outOk, outPending, outRefused, trainEnv, type TrainEnv, type TrainOut } from './ship-queue-train';
|
||||
import { checkWavePin } from './ship-queue-wave';
|
||||
import { readyPlan } from '../../lib/plan-record';
|
||||
@@ -183,8 +183,7 @@ function recordReceipt(env: TrainEnv, receipt: ReadyReceipt, lines: string[], ag
|
||||
const p = persist(env.ctx.repoRoot, [receipt], { remote: env.remote });
|
||||
lines.push(`READY_RECEIPT: ${file} (${p.detail})`);
|
||||
if (p.pushed) return null;
|
||||
return outRefused({ code: 'QUEUE_READY_NOTE_UNPUSHED', message: `the READY receipt for #${receipt.pr} at ${receipt.head.slice(0, 12)} is written locally, but ${env.remote} does not hold its note on refs/notes/gstack-trains (${p.detail}); land reads that note, so this head is not READY` },
|
||||
lines, `fix the notes push to ${env.remote}, then ${again}`, file);
|
||||
return outRefused(readyNoteUnpushed(receipt.pr, receipt.head, env.remote, p.detail), lines, `fix the notes push to ${env.remote}, then ${again}`, file);
|
||||
}
|
||||
|
||||
const screenLine = (c: ScreenChoice) => `screen: ${c.choice}${c.policy ? ` policy=${c.policy} nonce=${c.nonce} run=${c.run ?? '-'}${c.verdict ? ` verdict=${c.verdict.outcome}` : ''}` : ''}`;
|
||||
|
||||
@@ -324,6 +324,14 @@ export function persist(repoRoot: string, records: AnyRecord[], opts: { push?: b
|
||||
return { pushed: false, detail: `git push ${NOTES_REF} failed after 3 tries: ${last}` };
|
||||
}
|
||||
|
||||
/**
|
||||
* QUEUE_READY_NOTE_UNPUSHED: `land --pin` reads the READY note on the remote, so a READY receipt written only here is
|
||||
* not READY (reports/GSTA-461.md item 19). The train and plain ready paths both refuse with it.
|
||||
*/
|
||||
export function readyNoteUnpushed(pr: string, head: string, remote: string, detail: string): { code: 'QUEUE_READY_NOTE_UNPUSHED'; message: string } {
|
||||
return { code: 'QUEUE_READY_NOTE_UNPUSHED', message: `the READY receipt for #${pr} at ${head.slice(0, 12)} is written locally, but ${remote} does not hold its note on ${NOTES_REF} (${detail}); land reads that note, so this head is not READY` };
|
||||
}
|
||||
|
||||
function newerOrMissing(r: AnyRecord): boolean {
|
||||
try {
|
||||
const cur = JSON.parse(fs.readFileSync(recordPath(r), 'utf8'));
|
||||
|
||||
+17
-3
@@ -223,7 +223,10 @@ READY_RECEIPT: <state root>/ship-queue/ready/<repo key>/42-<head>.json (notes pu
|
||||
```
|
||||
|
||||
Under `changelog_fragments`, a `READY_RECEIPT:` line follows, written only when
|
||||
every check above passed; `land` refuses a pin without one.
|
||||
every check above passed; `land` refuses a pin without one. It says `notes
|
||||
pushed` only once origin holds the note; otherwise `ready` refuses with
|
||||
[`QUEUE_READY_NOTE_UNPUSHED`](troubleshooting.md#queue-ready-note-unpushed),
|
||||
as it does under a train, because `land` reads the note on origin.
|
||||
|
||||
CI's Version Gate (`scripts/compare-pr-version.ts`) accepts that head as
|
||||
unstamped when `VERSION` and `package.json`'s version match its merge base, the
|
||||
@@ -276,6 +279,14 @@ counts as a failed check `<workflow> (run <id>, no jobs)` unless a later run of
|
||||
the same workflow replaced it; a run cancelled before any job is not counted.
|
||||
An unreadable list is unknown, never green.
|
||||
|
||||
When a check counts as passing only because a newer run from another check
|
||||
suite replaced a red one (a branch-only `free-tests.yml` dispatch beside the red
|
||||
`pull_request` run, which tested the merge with the base), `ready` prints
|
||||
`CI_SUPERSEDED: <check> red run <id> replaced by <id> (suite <a> → <b>)` under
|
||||
the `ci:` line, one per replaced run, and `land` prints the same under its `CI:`
|
||||
line. It is a warning and changes no verdict; a rerun in the same suite prints
|
||||
nothing ([what to do](troubleshooting.md#ci-superseded)).
|
||||
|
||||
It refuses (`QUEUE_NOT_READY`) when the working tree has tracked edits, HEAD
|
||||
is not the PR's pushed head, CI at that exact head is not green, or the pregate
|
||||
result is not a current pass, and (`QUEUE_RECEIPT_MISSING`,
|
||||
@@ -732,9 +743,12 @@ slot. The queue enforces this at every step from the PR body alone:
|
||||
`REVIEW_TARGETED:` line) when a targeted class the head needs has no done
|
||||
targeted pass in the receipt, and `QUEUE_RECEIPT_EXCEPTION_INVALID` when an
|
||||
owner exception it records does not stand ([the ship receipt](#ship-receipt)).
|
||||
There is no flag around it. Before any of these checks, `ready` fetches
|
||||
There is no flag around it. Before any of these checks, and before it
|
||||
chooses the train or plain path from the policy, `ready` fetches
|
||||
`origin/<base>` and reads the policy and every receipt field against that
|
||||
fresh ref, so a lane clone whose base is behind the remote never turns a good
|
||||
fresh ref (a clone with a missing or stale base ref would otherwise run plain
|
||||
`ready` under a train policy and skip the screen, conflict and READY-receipt
|
||||
steps), so a lane clone whose base is behind the remote never turns a good
|
||||
receipt into `RECEIPT_INVALID` (GST-2232); a failed fetch refuses with
|
||||
`QUEUE_BASE_UNFETCHED` before anything is compared. The stamp path of `land`
|
||||
refuses the same way; `land --pin` (`merge: direct`) refuses a failed fetch
|
||||
|
||||
+26
-4
@@ -5503,8 +5503,8 @@ resumes its own lock.
|
||||
### `QUEUE_BASE_UNFETCHED`
|
||||
|
||||
**Meaning.** `gstack-ship-queue ready` (or the stamp path of `land`) could not
|
||||
fetch `origin/<base>`, so it stopped before reading the policy or comparing
|
||||
any receipt field. Both compare the receipt's recorded selection, depth and
|
||||
fetch `origin/<base>`, so it stopped before reading the policy (which also
|
||||
picks `ready`'s train or plain path) or comparing any receipt field. Both compare the receipt's recorded selection, depth and
|
||||
targeted classes with what the current base gives the head; a local
|
||||
`origin/<base>` a few landings old measures the PR from an older merge base,
|
||||
pulls in files of PRs that already landed, and reports a false
|
||||
@@ -6525,10 +6525,30 @@ CHANGELOG drops or renames a released heading (the message then names the
|
||||
CHANGELOG). `train prune --stale` deletes the scratch branches of merged or
|
||||
closed PRs.
|
||||
|
||||
<a id="ci-superseded"></a>
|
||||
### `CI_SUPERSEDED: <check> red run <id> replaced by <id> (suite <a> → <b>)`
|
||||
|
||||
**Meaning.** `gstack-ship-queue ready` or `land` judges each check by its
|
||||
newest run, and here the newest run passed in a different check suite from an
|
||||
older red run of the same check. The usual cause is a lane dispatching
|
||||
`free-tests.yml` on its branch after the `pull_request` run went red. The
|
||||
`pull_request` run tested GitHub's merge of the branch with the base, and the
|
||||
dispatched run tested the branch alone, so a failure that exists only on the
|
||||
merge with main can be hidden. The line is a warning: CI still counts as
|
||||
green, and nothing is refused. A rerun of the red run in its own suite prints
|
||||
nothing (reports/GSTA-461.md item 7).
|
||||
|
||||
**Fix.** Nothing is required. Before sending READY or landing, open the red
|
||||
run (`gh run view <id>` names the job) and check whether it failed for a reason
|
||||
the branch-only run could not see. If it was a flaky test, log it as flaky the
|
||||
usual way. To clear the line, rerun the failed `pull_request` run itself
|
||||
(`gh run rerun <run> --failed`), which replaces it in the same suite.
|
||||
|
||||
<a id="queue-ready-note-unpushed"></a>
|
||||
### `QUEUE_READY_NOTE_UNPUSHED`
|
||||
|
||||
**Meaning.** `gstack-ship-queue ready` wrote the READY receipt in the local
|
||||
**Meaning.** `gstack-ship-queue ready` (under a train policy, or under
|
||||
`changelog_fragments` without one) wrote the READY receipt in the local
|
||||
state root, but origin does not hold its note on `refs/notes/gstack-trains`.
|
||||
`ready` pushes the notes ref, then fetches it back and requires origin's tip
|
||||
to be the local tip or to contain it. It tries three times, fetching and
|
||||
@@ -6543,7 +6563,9 @@ the land failed instead (#4245). Don't send the READY block.
|
||||
refused it, then run the `NEXT:` line, which is the same `ready` command
|
||||
without `--pregate`, `--gate-exit` or `--gate-log`: add back any of those you
|
||||
passed the first time. It keeps the first `ready_at`, follows a screen run already dispatched at the
|
||||
same merge SHA, and pushes the kept receipt. To see the raw error, run
|
||||
same merge SHA, and pushes the kept receipt. Without a train policy there is
|
||||
no `NEXT:` line: rerun the same `ready` command with the same flags, and it
|
||||
writes the receipt again and pushes it. To see the raw error, run
|
||||
`git push origin refs/notes/gstack-trains:refs/notes/gstack-trains`.
|
||||
|
||||
<a id="queue-screen-red"></a>
|
||||
|
||||
@@ -189,7 +189,7 @@ export function landDirect(ctx: QueueContext, o: DirectOptions): StepOutput {
|
||||
}
|
||||
|
||||
const ci = ciAtHead(ctx, o.pin);
|
||||
lines.push(`CI: ${ci.line}`);
|
||||
lines.push(`CI: ${ci.line}`, ...ci.superseded);
|
||||
if (!ci.green) return fail(lines, { code: 'QUEUE_LAND_CHECKS', message: `checks are ${ci.line}`, fix: `wait for pending checks (gstack-pr-wait ${o.pr} --until green), or fix the red one at a new head; then rerun land` });
|
||||
const passed = new Set([...ci.checks.filter(c => c.conclusion === 'success').map(c => c.name), ...ci.contexts.filter(c => c.state === 'success').map(c => c.name)]);
|
||||
const missing = (policy.changelog_fragments?.required_checks ?? []).filter(n => !passed.has(n));
|
||||
|
||||
@@ -357,7 +357,7 @@ export function land(ctx: QueueContext, o: LandOptions): StepOutput {
|
||||
}
|
||||
if (!j) {
|
||||
const ci = ciAtHead(ctx, pin);
|
||||
lines.push(`CI: ${ci.line}`);
|
||||
lines.push(`CI: ${ci.line}`, ...ci.superseded);
|
||||
if (!ci.green) return fail(lines, { code: 'QUEUE_NOT_READY', message: `required checks are not green at the pin ${pin.slice(0, 12)}` }, run);
|
||||
}
|
||||
if (o.dryRun) {
|
||||
|
||||
+55
-17
@@ -41,7 +41,7 @@ import {
|
||||
type AttemptRecord, type QueueFailure,
|
||||
} from '../crew/lib/ship-queue-gate';
|
||||
import { wtreeFingerprint } from './script-invocation';
|
||||
import { nowIso, persist, readyPath, repoIdentity } from '../crew/lib/ship-queue-train-record';
|
||||
import { nowIso, persist, readyNoteUnpushed, readyPath, repoIdentity } from '../crew/lib/ship-queue-train-record';
|
||||
import { buildReceipt, type TreeReceipt } from './tree-receipt';
|
||||
|
||||
export type Gh = (args: string[]) => { ok: boolean; out: string; err?: string };
|
||||
@@ -134,22 +134,46 @@ function readAllPages(ctx: QueueContext, endpoint: string, key: 'check_runs' | '
|
||||
* completed run with no readable completed_at, is never collapsed and counts on
|
||||
* its own (fail closed).
|
||||
*/
|
||||
export function latestRunPerName<T extends { name?: unknown; id?: unknown; status?: unknown; completed_at?: unknown; app?: unknown }>(runs: T[]): T[] {
|
||||
type CheckRun = { name?: unknown; id?: unknown; status?: unknown; completed_at?: unknown; app?: unknown };
|
||||
const runRank = (r: CheckRun) => (r.status !== 'completed' ? Infinity : typeof r.completed_at === 'string' ? Date.parse(r.completed_at) : NaN);
|
||||
/** One app's check name, or null for a run that is never collapsed (no name, or completed with no readable completed_at). */
|
||||
function checkKey(r: CheckRun): string | null {
|
||||
if (typeof r.name !== 'string' || !r.name || Number.isNaN(runRank(r))) return null;
|
||||
const app = (r.app as { id?: unknown } | null | undefined)?.id;
|
||||
return `${typeof app === 'number' || typeof app === 'string' ? app : ''}\0${r.name}`;
|
||||
}
|
||||
|
||||
export function latestRunPerName<T extends CheckRun>(runs: T[]): T[] {
|
||||
const latest = new Map<string, T>();
|
||||
const standalone: T[] = [];
|
||||
const rank = (r: T) => (r.status !== 'completed' ? Infinity : typeof r.completed_at === 'string' ? Date.parse(r.completed_at) : NaN);
|
||||
const id = (r: T) => (typeof r.id === 'number' ? r.id : Number(r.id) || 0);
|
||||
for (const r of runs) {
|
||||
const app = (r.app as { id?: unknown } | null | undefined)?.id;
|
||||
if (typeof r.name !== 'string' || !r.name || Number.isNaN(rank(r))) { standalone.push(r); continue; }
|
||||
const key = `${typeof app === 'number' || typeof app === 'string' ? app : ''}\0${r.name}`;
|
||||
const key = checkKey(r);
|
||||
if (key === null) { standalone.push(r); continue; }
|
||||
const have = latest.get(key);
|
||||
if (!have || rank(r) > rank(have) || (rank(r) === rank(have) && id(r) > id(have))) latest.set(key, r);
|
||||
if (!have || runRank(r) > runRank(have) || (runRank(r) === runRank(have) && id(r) > id(have))) latest.set(key, r);
|
||||
}
|
||||
return [...latest.values(), ...standalone];
|
||||
}
|
||||
|
||||
const suiteOf = (r: Record<string, unknown>) => String((r.check_suite as { id?: unknown } | null | undefined)?.id ?? '');
|
||||
const PASSING = ['success', 'skipped', 'neutral'];
|
||||
|
||||
/**
|
||||
* A CI_SUPERSEDED line for each red run whose check counts by a passing newer run from another check suite, such as a
|
||||
* branch-only dispatch that replaced a red pull_request run, which tested the merge with the base (reports/GSTA-461.md
|
||||
* item 7). It only warns: the verdict is latestRunPerName's. A rerun in the same suite prints nothing.
|
||||
*/
|
||||
function supersededLines(runs: Array<Record<string, unknown>>, judged: Array<Record<string, unknown>>): string[] {
|
||||
const winners = new Map(judged.map(r => [checkKey(r), r] as const).filter(([k]) => k !== null));
|
||||
return runs.flatMap(r => {
|
||||
const key = checkKey(r);
|
||||
const by = key === null ? undefined : winners.get(key);
|
||||
if (!by || by === r || r.status !== 'completed' || PASSING.includes(String(r.conclusion))) return [];
|
||||
if (by.status !== 'completed' || !PASSING.includes(String(by.conclusion)) || suiteOf(r) === suiteOf(by)) return [];
|
||||
return [`CI_SUPERSEDED: ${String(r.name)} red run ${String(r.id ?? '?')} replaced by ${String(by.id ?? '?')} (suite ${suiteOf(r) || '?'} → ${suiteOf(by) || '?'})`];
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Workflow runs at a head that ended without a passing conclusion and without one check run (a startup_failure, or a
|
||||
@@ -183,19 +207,20 @@ export function joblessRuns(latestSuites: Set<string>, judged: Array<Record<stri
|
||||
* judged by that list alone, so a rerun whose matrix dropped a failed job retires it; a suite only filter=all names (its
|
||||
* latest attempt had no jobs) is judged by its earlier attempts. latestRunPerName then picks the newest run per name.
|
||||
*/
|
||||
export function ciAtHead(ctx: QueueContext, sha: string): { line: string; green: boolean; none: boolean; checks: Array<{ name: string; status: string; conclusion: string | null }>; contexts: Array<{ name: string; state: string }> } {
|
||||
export function ciAtHead(ctx: QueueContext, sha: string): { line: string; green: boolean; none: boolean; checks: Array<{ name: string; status: string; conclusion: string | null }>; contexts: Array<{ name: string; state: string }>; superseded: string[] } {
|
||||
const at = sha.slice(0, 12);
|
||||
const latest = readAllPages(ctx, `repos/{owner}/{repo}/commits/${sha}/check-runs`, 'check_runs');
|
||||
if ('error' in latest) return { line: `unknown at ${at} (${latest.error})`, green: false, none: false, checks: [], contexts: [] };
|
||||
if ('error' in latest) return { line: `unknown at ${at} (${latest.error})`, green: false, none: false, checks: [], contexts: [], superseded: [] };
|
||||
const all = readAllPages(ctx, `repos/{owner}/{repo}/commits/${sha}/check-runs?filter=all`, 'check_runs');
|
||||
if ('error' in all) return { line: `unknown at ${at} (${all.error})`, green: false, none: false, checks: [], contexts: [] };
|
||||
if ('error' in all) return { line: `unknown at ${at} (${all.error})`, green: false, none: false, checks: [], contexts: [], superseded: [] };
|
||||
const statuses = readAllPages(ctx, `repos/{owner}/{repo}/commits/${sha}/status`, 'statuses');
|
||||
if ('error' in statuses) return { line: `unknown at ${at} (${statuses.error})`, green: false, none: false, checks: [], contexts: [] };
|
||||
if ('error' in statuses) return { line: `unknown at ${at} (${statuses.error})`, green: false, none: false, checks: [], contexts: [], superseded: [] };
|
||||
const workflowRuns = readAllPages(ctx, `repos/{owner}/{repo}/actions/runs?head_sha=${sha}`, 'workflow_runs');
|
||||
if ('error' in workflowRuns) return { line: `unknown at ${at} (${workflowRuns.error})`, green: false, none: false, checks: [], contexts: [] };
|
||||
if ('error' in workflowRuns) return { line: `unknown at ${at} (${workflowRuns.error})`, green: false, none: false, checks: [], contexts: [], superseded: [] };
|
||||
const latestSuites = new Set(latest.items.map(suiteOf).filter(Boolean));
|
||||
const latestIds = new Set(latest.items.map(r => String(r.id ?? '')).filter(Boolean));
|
||||
const judged = latestRunPerName([...latest.items, ...all.items.filter(r => suiteOf(r) ? !latestSuites.has(suiteOf(r)) : !!r.id && !latestIds.has(String(r.id)))]);
|
||||
const candidates = [...latest.items, ...all.items.filter(r => suiteOf(r) ? !latestSuites.has(suiteOf(r)) : !!r.id && !latestIds.has(String(r.id)))];
|
||||
const judged = latestRunPerName(candidates);
|
||||
const checks = [
|
||||
...judged.map(c => ({ name: String(c.name ?? ''), status: String(c.status ?? ''), conclusion: c.conclusion === undefined || c.conclusion === null ? null : String(c.conclusion) })),
|
||||
...joblessRuns(latestSuites, judged, workflowRuns.items),
|
||||
@@ -207,7 +232,7 @@ export function ciAtHead(ctx: QueueContext, sha: string): { line: string; green:
|
||||
const count = (v: string) => verdicts.filter(x => x === v).length;
|
||||
const [passed, failed, pending, skipped] = [count('passed'), count('failed'), count('pending'), count('skipped')];
|
||||
const green = verdicts.length > 0 && pending === 0 && failed === 0;
|
||||
return { line: `${green ? 'green' : pending ? 'pending' : verdicts.length ? 'red' : 'none'} at ${at} (passed=${passed} failed=${failed} pending=${pending} skipped=${skipped})`, green, none: verdicts.length === 0, checks, contexts: statuses.items.map(s => ({ name: String(s.context ?? ''), state: String(s.state ?? '') })) };
|
||||
return { line: `${green ? 'green' : pending ? 'pending' : verdicts.length ? 'red' : 'none'} at ${at} (passed=${passed} failed=${failed} pending=${pending} skipped=${skipped})`, green, none: verdicts.length === 0, checks, contexts: statuses.items.map(s => ({ name: String(s.context ?? ''), state: String(s.state ?? '') })), superseded: supersededLines(candidates, judged) };
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -256,11 +281,21 @@ function gateLine(policy: LoadedPolicy, attempt: AttemptRecord | null): string {
|
||||
return `gate: command=${p.gate_command ? JSON.stringify(p.gate_command) : 'none'} full_e2e=${p.full_e2e ? 'yes' : 'no'} gate-ahead=${rec}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Refresh origin/<base>: ready reads the base policy (which also picks the train or plain path) and binds the receipt
|
||||
* against the current base, never a ref it did not refresh.
|
||||
*/
|
||||
export function refreshBase(ctx: QueueContext): QueueFailure | null {
|
||||
const fetched = git(ctx.repoRoot, ['fetch', '--no-tags', '-q', 'origin', `+refs/heads/${ctx.base}:refs/remotes/origin/${ctx.base}`]);
|
||||
if (fetched.ok) return null;
|
||||
return { code: 'QUEUE_BASE_UNFETCHED', message: `cannot fetch origin/${ctx.base} (${fetched.err.trim().split('\n')[0]?.slice(0, 200) ?? ''}); ready reads the policy and checks the receipt against the current base, never a ref it did not refresh`, fix: `fix the fetch (git fetch origin ${ctx.base}), then rerun ready` };
|
||||
}
|
||||
|
||||
/** READY: one block, identifiers first, about the exact pushed PR head. */
|
||||
export function ready(ctx: QueueContext, o: { pr: string; pregate?: string; gateExit?: number; gateLog?: string }): StepOutput {
|
||||
const run = PREGATE_OUT_REL;
|
||||
const fetched = git(ctx.repoRoot, ['fetch', '--no-tags', '-q', 'origin', `+refs/heads/${ctx.base}:refs/remotes/origin/${ctx.base}`]);
|
||||
if (!fetched.ok) return fail({ code: 'QUEUE_BASE_UNFETCHED', message: `cannot fetch origin/${ctx.base} (${fetched.err.trim().split('\n')[0]?.slice(0, 200) ?? ''}); ready reads the policy and checks the receipt against the current base, never a ref it did not refresh`, fix: `fix the fetch (git fetch origin ${ctx.base}), then rerun ready` }, [], run);
|
||||
const unfetched = refreshBase(ctx);
|
||||
if (unfetched) return fail(unfetched, [], run);
|
||||
const policy = loadBasePolicy(ctx);
|
||||
if ('message' in policy) return fail(policy, [], run);
|
||||
const head = git(ctx.repoRoot, ['rev-parse', 'HEAD']).out;
|
||||
@@ -318,6 +353,7 @@ export function ready(ctx: QueueContext, o: { pr: string; pregate?: string; gate
|
||||
`version: ${version}`,
|
||||
`base: origin/${ctx.base}@${baseSha}`,
|
||||
`ci: ${ci.line}`,
|
||||
...ci.superseded,
|
||||
...(hint ? [hint] : []),
|
||||
pregateLine,
|
||||
...(accepted ? flakyExceptionLines(accepted) : []),
|
||||
@@ -340,8 +376,10 @@ export function ready(ctx: QueueContext, o: { pr: string; pregate?: string; gate
|
||||
if (!policy.policy.train && policy.policy.changelog_fragments) {
|
||||
const repo = repoIdentity(ctx.repoRoot);
|
||||
const now = nowIso();
|
||||
const p = persist(ctx.repoRoot, [{ schema: 1, kind: 'ready', repo_key: repo.key, pr: o.pr, head, base_sha: baseSha, ready_at: now, screen: { choice: 'no-screen', policy: null, nonce: null, run: null, merge_sha: null }, updated_at: now, ...(plan.record ? { plan: plan.record } : {}) }], { remote: ctx.remote ?? 'origin' });
|
||||
const remote = ctx.remote ?? 'origin';
|
||||
const p = persist(ctx.repoRoot, [{ schema: 1, kind: 'ready', repo_key: repo.key, pr: o.pr, head, base_sha: baseSha, ready_at: now, screen: { choice: 'no-screen', policy: null, nonce: null, run: null, merge_sha: null }, updated_at: now, ...(plan.record ? { plan: plan.record } : {}) }], { remote });
|
||||
lines.push(`READY_RECEIPT: ${readyPath(repo.key, o.pr, head)} (${p.detail})`);
|
||||
if (!p.pushed) return fail({ ...readyNoteUnpushed(o.pr, head, remote, p.detail), fix: `fix the notes push to ${remote} (the READY_RECEIPT line ends with its error), then rerun gstack-ship-queue ready --pr ${o.pr} with the same flags` }, lines, run);
|
||||
}
|
||||
return { ok: true, lines, run };
|
||||
}
|
||||
|
||||
@@ -689,6 +689,35 @@ describe('ready reads all of CI at the head (review #3235 P1-5)', () => {
|
||||
expect(plain.r.status).toBe(EXIT.ok);
|
||||
}, 240_000);
|
||||
|
||||
// Value: protects=ready names each red run that a newer run from another check suite replaced (a branch-only dispatch beside the red pull_request
|
||||
// run) and keeps the verdict; fails_when=free-tests red in suite 1 and green in suite 3 prints green with no CI_SUPERSEDED line, or a rerun in the
|
||||
// same suite, a pass replacing a pass, or a red newest run prints one; why_new=reports/GSTA-461.md item 7; seam=ciAtHead's latestRunPerName through
|
||||
// the gh shim; no_claim=which suite tested the merge with main
|
||||
test('a red run that a newer run from another check suite replaced prints CI_SUPERSEDED and changes no verdict (GSTA-461 item 7)', () => {
|
||||
const app = { id: 15368 };
|
||||
const run = (id: number, name: string, conclusion: string, completed_at: string, suite: number, earlier = false) => ({ id, name, status: 'completed', conclusion, completed_at, app, check_suite: { id: suite }, ...(earlier ? { earlier_attempt: true } : {}) });
|
||||
const title = run(50, 'Plain title', 'success', '2026-11-09T10:05:00Z', 2);
|
||||
const superseded = (out: string) => out.split('\n').filter(l => l.startsWith('CI_SUPERSEDED:'));
|
||||
const replaced = readyWith({ check_runs: [run(10, 'free-tests', 'failure', '2026-11-09T10:00:00Z', 1), run(30, 'free-tests', 'success', '2026-11-09T10:20:00Z', 3), title] });
|
||||
expect(replaced.r.status).toBe(EXIT.ok);
|
||||
const lines = replaced.r.out.split('\n');
|
||||
const ci = lines.indexOf(`ci: green at ${replaced.at} (passed=2 failed=0 pending=0 skipped=0)`);
|
||||
expect(ci).toBeGreaterThan(0);
|
||||
expect(lines[ci + 1]).toBe('CI_SUPERSEDED: free-tests red run 10 replaced by 30 (suite 1 → 3)');
|
||||
expect(superseded(replaced.r.out)).toHaveLength(1);
|
||||
const two = readyWith({ check_runs: [run(30, 'free-tests', 'success', '2026-11-09T10:20:00Z', 3), run(10, 'free-tests', 'failure', '2026-11-09T10:00:00Z', 1), run(11, 'Plain title', 'timed_out', '2026-11-09T10:01:00Z', 1), run(51, 'Plain title', 'success', '2026-11-09T10:30:00Z', 4)] });
|
||||
expect(two.r.status).toBe(EXIT.ok);
|
||||
expect(superseded(two.r.out)).toEqual(['CI_SUPERSEDED: free-tests red run 10 replaced by 30 (suite 1 → 3)', 'CI_SUPERSEDED: Plain title red run 11 replaced by 51 (suite 1 → 4)']);
|
||||
const sameSuite = readyWith({ check_runs: [run(10, 'free-tests', 'failure', '2026-11-09T10:00:00Z', 1, true), run(30, 'free-tests', 'success', '2026-11-09T10:20:00Z', 1), title] });
|
||||
expect(sameSuite.r.status).toBe(EXIT.ok);
|
||||
expect(superseded(sameSuite.r.out)).toEqual([]);
|
||||
const passOverPass = readyWith({ check_runs: [run(10, 'free-tests', 'success', '2026-11-09T10:00:00Z', 1), run(30, 'free-tests', 'success', '2026-11-09T10:20:00Z', 3), title] });
|
||||
expect(superseded(passOverPass.r.out)).toEqual([]);
|
||||
const redNewest = readyWith({ check_runs: [run(10, 'free-tests', 'success', '2026-11-09T10:00:00Z', 1), run(30, 'free-tests', 'failure', '2026-11-09T10:20:00Z', 3), title] });
|
||||
expect(redNewest.r.status).toBe(EXIT.refused);
|
||||
expect(superseded(redNewest.r.out)).toEqual([]);
|
||||
}, 240_000);
|
||||
|
||||
// Value: protects=the workflow-runs read fails closed like the check-runs read; fails_when=ready treats an unreadable or short actions/runs list
|
||||
// as zero jobless runs and calls the head green; seam=ciAtHead's readAllPages over actions/runs?head_sha; no_claim=GitHub's own pagination
|
||||
test('an unreadable or short workflow-runs list is unknown, never green', () => {
|
||||
|
||||
@@ -27,9 +27,11 @@ import { readTrainNotes } from '../crew/scripts/release-cut';
|
||||
import { gitIn } from '../crew/scripts/train-gate';
|
||||
import type { GhRunner } from '../crew/scripts/recover-receipts';
|
||||
import { TRAIN_BLOCK } from './helpers/ship-queue-train-fixture';
|
||||
import { CHECK_IDS } from '../lib/pregate/index';
|
||||
|
||||
const REQUIRED = ['free-tests', 'Check VERSION is not stale vs queue'];
|
||||
const POLICY = { ...BASE_POLICY, release_tool: null, release_outputs: [], changelog_fragments: { dir: 'changelog.d', required_checks: REQUIRED }, merge: 'direct', min_gstack_version: MERGE_KEY_VERSION };
|
||||
const FULL_CHECKS = CHECK_IDS.map(id => ({ id, status: 'pass' }));
|
||||
/** The base policy with land.merged_checks: the first proves the checks see the merged tree, the second stands in for a budget cap a stale base can break (#3886). */
|
||||
const CHECKED = { ...POLICY, land: { merged_checks: ['test -e other.txt && ls pr-*.txt >/dev/null', "test ! -e budget.md || test ! -e other.txt || { echo 'budget.md over its cap' >&2; exit 1; }"] }, min_gstack_version: LAND_KEY_VERSION };
|
||||
const repos: FixtureRepo[] = [];
|
||||
@@ -59,7 +61,7 @@ function setup(policy: Record<string, unknown> = POLICY): Fx {
|
||||
" if(m[2]==='status'){console.log(JSON.stringify({total_count:0,statuses:[]}));process.exit(0)}",
|
||||
" const set=d.checks[m[1]]??Object.fromEntries(d.defaultChecks.map(n=>[n,'success']));",
|
||||
" const all=/[?&]filter=all(&|$)/.test(a[1]);",
|
||||
" const runs=Object.entries(set).filter(([,v])=>all||!v.earlierAttempt).map(([name,v],i)=>{const c=typeof v==='string'?v:v.conclusion;return {id:i+1,name,status:c==='pending'?'in_progress':'completed',conclusion:c==='pending'?null:c,completed_at:'2026-11-03T00:00:00Z',app:{id:1}}});",
|
||||
" const runs=Array.isArray(set)?set.filter(r=>all||!r.earlier_attempt):Object.entries(set).filter(([,v])=>all||!v.earlierAttempt).map(([name,v],i)=>{const c=typeof v==='string'?v:v.conclusion;return {id:i+1,name,status:c==='pending'?'in_progress':'completed',conclusion:c==='pending'?null:c,completed_at:'2026-11-03T00:00:00Z',app:{id:1}}});",
|
||||
" console.log(JSON.stringify({total_count:runs.length,check_runs:runs}));process.exit(0)}",
|
||||
"if(a[0]==='pr'&&a[1]==='merge'){const p=d.prs[a[2]];const want=a[a.indexOf('--match-head-commit')+1];const head=g('rev-parse','refs/heads/'+p.headRefName);",
|
||||
" if(head!==want){console.error('Head branch was modified');process.exit(1)}",
|
||||
@@ -477,6 +479,61 @@ describe('land --pin: direct merge at the PR\'s own green head', () => {
|
||||
}, 90_000);
|
||||
});
|
||||
|
||||
describe('land --pin and ready under merge: direct (GSTA-461 items 7 and 19)', () => {
|
||||
const run = (id: number, name: string, conclusion: string, completed_at: string, suite: number) => ({ id, name, status: 'completed', conclusion, completed_at, app: { id: 1 }, check_suite: { id: suite } });
|
||||
|
||||
// Value: protects=land --pin names each red run that a newer run from another check suite replaced, right after its CI line, and still lands;
|
||||
// fails_when=land prints plain green and merges with no word when free-tests failed in suite 1 and a branch-only run passed in suite 3;
|
||||
// why_new=reports/GSTA-461.md item 7; seam=the gh shim's raw check-run list; no_claim=the same-suite rule that would refuse (after the freeze)
|
||||
test('a red check replaced by a pass from another check suite lands with a CI_SUPERSEDED line', () => {
|
||||
const fx = setup();
|
||||
const pin = openPr(fx, '9');
|
||||
fx.write(d => { d.checks[pin] = [run(1, 'free-tests', 'failure', '2026-11-09T10:00:00Z', 1), run(2, 'Check VERSION is not stale vs queue', 'success', '2026-11-09T10:02:00Z', 2), run(3, 'free-tests', 'success', '2026-11-09T10:20:00Z', 3)]; });
|
||||
const r = land(fx, '9', pin);
|
||||
expect(r.err).toBe('');
|
||||
expect(r.status).toBe(EXIT.ok);
|
||||
const lines = r.out.split('\n');
|
||||
const ci = lines.findIndex(l => l.startsWith('CI: green at '));
|
||||
expect(ci).toBeGreaterThanOrEqual(0);
|
||||
expect(lines[ci + 1]).toBe('CI_SUPERSEDED: free-tests red run 1 replaced by 3 (suite 1 → 3)');
|
||||
expect(r.out).toContain('MERGED: #9 as ');
|
||||
}, 90_000);
|
||||
|
||||
// Value: protects=ready without a train block refuses when origin never receives its READY note, and a rerun after the fix pushes the kept note
|
||||
// that land --pin then reads from origin; fails_when=ready prints a READY_RECEIPT line and exits 0 while origin lacks the note (our own land on
|
||||
// 11-09 refused QUEUE_NOT_READY this way); why_new=#4254 covers only the train path; seam=a pre-receive hook in the fixture's bare origin, and a
|
||||
// separate state root for ready so land can only find the note on origin; no_claim=GitHub's own rejection messages
|
||||
test('origin rejects refs/notes/*: ready refuses QUEUE_READY_NOTE_UNPUSHED; once origin accepts, a rerun pushes the note and land merges', () => {
|
||||
const fx = setup();
|
||||
const head = openPr(fx, '31', { ready: false });
|
||||
fx.repo.git('checkout', '-q', 'pr-31');
|
||||
const pregate = path.join(fx.repo.root, 'pregate.json');
|
||||
fs.writeFileSync(pregate, JSON.stringify({ schema_version: 1, tree: runBin(fx.repo, 'gstack-wtree', []).out.trim(), head, verdict: 'pass', checks: FULL_CHECKS }));
|
||||
const env = { ...fx.env, GSTACK_STATE_ROOT: path.join(fx.repo.root, 'ready-state') };
|
||||
const ready = () => runBin(fx.repo, 'gstack-ship-queue', ['ready', '--pr', '31', '--pregate', pregate, '--base', 'main'], env);
|
||||
const hook = path.join(fx.repo.origin, 'hooks', 'pre-receive');
|
||||
const remoteNotes = () => fx.repo.git('ls-remote', 'origin', 'refs/notes/gstack-trains').split(/\s+/)[0] ?? '';
|
||||
fs.writeFileSync(hook, '#!/bin/sh\nwhile read old new ref; do case "$ref" in refs/notes/*) echo "notes refused by fixture" >&2; exit 1;; esac; done; exit 0\n', { mode: 0o755 });
|
||||
const refused = ready();
|
||||
expect(refused.status).toBe(EXIT.refused);
|
||||
expect(refused.err).toContain('(QUEUE_READY_NOTE_UNPUSHED)');
|
||||
expect(refused.err).toContain('gstack-ship-queue ready --pr 31');
|
||||
expect(refused.out).not.toContain('notes pushed');
|
||||
expect(refused.out).toMatch(/^GSTACK_RESULT: .*status=refused/m);
|
||||
expect(remoteNotes()).toBe('');
|
||||
fs.rmSync(hook);
|
||||
const again = ready();
|
||||
expect(again.err).toBe('');
|
||||
expect(again.status).toBe(EXIT.ok);
|
||||
expect(again.out).toMatch(/^READY_RECEIPT: .* \(notes pushed to origin refs\/notes\/gstack-trains\)$/m);
|
||||
expect(remoteNotes()).not.toBe('');
|
||||
fx.repo.git('checkout', '-q', '--detach', 'origin/main');
|
||||
const r = land(fx, '31', head);
|
||||
expect(r.status).toBe(EXIT.ok);
|
||||
expect(r.out).toContain('MERGED: #31 as ');
|
||||
}, 120_000);
|
||||
});
|
||||
|
||||
describe('land --pin: refusals (nothing merged)', () => {
|
||||
const refused = (fx: Fx, r: { status: number | null; err: string }, code: string) => {
|
||||
expect(r.status).not.toBe(EXIT.ok);
|
||||
|
||||
@@ -12,7 +12,7 @@ import { CHECK_IDS } from '../lib/pregate/index';
|
||||
import { REVIEW_EXCLUDE_KEY_VERSION } from '../lib/ship-policy';
|
||||
import { BASE_POLICY, cleanup, makeFixtureRepo, runBin, type FixtureRepo } from './helpers/restamp-fixture';
|
||||
import { ghQueueShim } from './helpers/ship-queue-fixture';
|
||||
import { shippedBody } from './helpers/ship-queue-train-fixture';
|
||||
import { openPr, queue as trainQueue, shippedBody, trainFixture } from './helpers/ship-queue-train-fixture';
|
||||
|
||||
const repos: FixtureRepo[] = [];
|
||||
afterEach(() => { while (repos.length) cleanup(repos.pop()!); });
|
||||
@@ -103,4 +103,23 @@ describe('ready refreshes origin/<base> before comparing receipt fields (GST-223
|
||||
expect([after.status, after.err]).toEqual([EXIT.ok, '']);
|
||||
expect(after.out).toContain(`base: origin/main@${moved}`);
|
||||
}, 180_000);
|
||||
|
||||
// Value: protects=ready chooses the train or plain path from the policy on a freshly fetched base, so a clone with no origin/main
|
||||
// ref never runs plain ready under a train policy (which skips the screen, conflict and READY-receipt steps);
|
||||
// fails_when=the bin reads the base policy before ready fetches it, finds none, prints that the policy declares no train block
|
||||
// and exits 0 with no READY_RECEIPT; why_new=#4256's fetch sits inside ready(), after the routing; seam=bin/gstack-ship-queue's
|
||||
// ready routing; no_claim=a base whose fetch fails (the case above refuses it)
|
||||
test('a clone with no origin/main ref routes ready by the fetched base policy: the train path, never plain ready', () => {
|
||||
const fx = trainFixture();
|
||||
repos.push(fx.repo);
|
||||
const head = openPr(fx, '1');
|
||||
fx.write(d => { d.ci[head] = [{ status: 'completed', conclusion: 'success' }]; });
|
||||
const pregate = path.join(fx.repo.root, 'pregate.json');
|
||||
fs.writeFileSync(pregate, JSON.stringify({ schema_version: 1, tree: runBin(fx.repo, 'gstack-wtree', []).out.trim(), head, verdict: 'pass', checks: FULL_CHECKS }));
|
||||
fx.repo.git('update-ref', '-d', 'refs/remotes/origin/main');
|
||||
const r = trainQueue(fx, ['ready', '--pr', '1', '--pregate', pregate, '--no-screen']);
|
||||
expect(r.err).not.toContain('declares no train block');
|
||||
expect([r.status, r.err]).toEqual([EXIT.ok, '']);
|
||||
expect(r.out).toMatch(/^READY_RECEIPT: .* \(notes pushed to origin refs\/notes\/gstack-trains\)$/m);
|
||||
}, 180_000);
|
||||
});
|
||||
Reference in new issue
Block a user