Repository navigation
checkGateRuns matches a kept failed run by sha256 only, ignoring its label #130
Description
- Repository:
pingdotgg/ts-rust - Target commit:
1f83d7fa96c15218f625220d6b886cdea0e7aa0e - File:
scripts/check-typechecker-batch.mjs:568
Summary
checkGateRuns validates a kept failed gate run by searching gateRuns for an entry whose new.sha256 equals the kept file's new.sha256; the kept file's label is parsed but never compared against the matched entry's label. A file named gate-compare-fail-r132-fail-2.json (label not present in gateRuns) therefore passes as long as some entry has the same hash, and the generated flake note credits r132-fail, excusing a repeat failure under a different label. The label is meant to be the identity of the gate run, so hash-only matching loses the association.
Affected code
scripts/check-typechecker-batch.mjs:
560
561 // True when a note's JSON text names name as a whole word, as accept_revision.py named() does: an
562 // item id does not match a longer id, and a run label may also be one segment of a path.
563 function namesWord(value, name, path = false) {
564 const [before, after] = path ? ["(?<![\\w.-])", "(?![\\w.-])"] : ["(?<![\\w./-])", "(?![\\w.-]|/\\w)"];
565 return new RegExp(before + name.replace(/[.*+?^${}()|[\]\\]/g, "\\$&") + after).test(JSON.stringify(value));
566 }
567
568 // Every gate run of the batch source (repeat-run rule). batch.gateRuns (accept_revision.py) lists
569 // them oldest first, the batch gate last: {label, manifest, sha256, compare {path, sha256},
570 // regressions [{id, base, new, why, flake}]}. candidate.sh side keeps each failed run's compare
571 // as gate-compare-fail-<label>.json next to gateCompare.output, and each of them must be in the
572 // list. The check runs gate-compare.py again on each earlier run. Each regressed item needs a
573 // saved flake note flake-r<revision>-<name> whose text names the item id and the run label, or it
574 // is a loss. Returns the flakes and the number of runs.
575 function checkGateRuns(state, batch, baseManifest, inputs, pin, tools, readEvidence, reasons) {
576 const runs = batch.gateRuns, last = Array.isArray(runs) ? runs.at(-1) : undefined;
577 requireValue(samePath(last?.manifest, batch.gate.manifest) && last.sha256 === batch.gate.sha256,
578 "gateRuns must list every gate run of the source, oldest first, with batch.gate last.");
579 requireValue(runs.every(run => text(run?.label) && Array.isArray(run.regressions)) && new Set(runs.map(run => run.label)).size === runs.length,
580 "Each gateRuns entry needs its own label and a regressions list.");
581 for (const path of tools.keptGateRuns(dirname(batch.gateCompare.output.path))) {
582 const kept = readEvidence({ path }, { pinned: false });
583 requireValue(runs.some(run => run.sha256 === kept?.new?.sha256), `The failed gate run ${path} of the source is not in gateRuns.`);
584 }
585 const prefix = `flake-r${batch.recoveryRevision}-`, notes = Object.keys(state).filter(name => name.startsWith(prefix)).sort();
586 const flakes = [];
587 for (const run of runs.slice(0, -1)) {
588 const manifest = readEvidence({ path: run.manifest, sha256: run.sha256 });
589 requireValue(sameInputs(tools.gitInputs(manifest.commit), inputs) && sameHash(manifest.upstreamPin, pin),
590 `Gate run ${run.label} is not a run of the batch source at the batch Go pin.`);
591 const compare = readEvidence(run.compare);
592 requireValue(compare?.new?.sha256 === run.sha256, `gateRuns ${run.label}: compare is not the gate-compare.py output of its manifest.`);
593 const { regressions } = tools.gateCompare(baseManifest, resolve(ROOT, run.manifest), batch.openDefects);
594 const ids = list => JSON.stringify(list.map(item => item?.id).sort());
595 requireValue(ids(run.regressions) === ids(regressions), `gateRuns ${run.label}: its regressions differ from gate-compare.py now.`);
596 for (const item of regressions) {
597 const note = notes.find(name => namesWord(state[name], item.id) && namesWord(state[name], run.label, true));
598 if (note) flakes.push({ run: run.label, id: item.id, note });
599 else reasons.push(`Gate run ${run.label}: ${item.id} ${item.base} -> ${item.new} (${item.why}) has no flake note ${prefix}<name> `
600 + "that names the item and the run.");
601 }
602 }
603 return { runs: runs.length, flakes };
604 }
605
606 // The Query core and Hono bound runs and the quality record of the batch source.
607 function checkRunEvidence(batch, readEvidence) {
608 for (const label of ["ordinaryQuery", "latestHono"]) {
609 const run = batch[label];
610 requireValue(run?.complete === true && run.exitCode === 0 && run.matchesOracle === true && run.sourceFingerprint === batch.sourceFingerprint
Expected behavior
Each kept gate-compare-fail-.json must be represented in gateRuns by that same run label, and flake-note lookup must use the kept run's label.
Actual behavior
In checkGateRuns, a kept failed run is matched to gateRuns only by runs.some(run => run.sha256 === kept?.new?.sha256); the kept file's label/path is not checked. Later, flake notes are matched against run.label. An entry with a different label but the same manifest sha therefore satisfies the kept-run check, and a flake note naming that different label can excuse the regression in the kept run.
How to reproduce
Call checkGateRuns with gateRuns ["r132-fail","r132"] and a kept file gate-compare-fail-r132-fail-2.json; it passes although the label is absent.
Root cause
checkGateRuns validates a kept failed gate run by searching gateRuns for an entry whose new.sha256 equals the kept file's new.sha256; the kept file's label is parsed but never compared against the matched entry's label. A file named gate-compare-fail-r132-fail-2.json (label not present in gateRuns) therefore passes as long as some entry has the same hash, and the generated flake note credits r132-fail, excusing a repeat failure under a different label. The label is meant to be the identity of the gate run, so hash-only matching loses the association.
Suggested fix
Check the kept file identity, not just its hash:
const label = kept.match(/gate-compare-fail-(.+)\.json$/)?.[1]
const entry = gateRuns.find(g => g.label === label)
if (!entry) stop(`... ${label} is not in gateRuns`)
if (entry.new?.sha256 !== kept.new?.sha256) stop(...)Add a test where the label is absent from gateRuns but the sha matches, and assert the run stops.
Activity