From aafbf7aabc44c1708c925276f04d2d50cd99b696 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 16 Jul 2026 19:02:52 -0700 Subject: [PATCH] fix(pr): close the review follow-ups on early gate proof (#109513) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(pr): close the review follow-ups on early gate proof Three fail-safe gaps from #109331's review that silently disabled the speedup: a stale terminal failure blocked a gate-proven pending rerun (now superseded only by a SCHEDULED run whose own gate passed — manual runs still cannot mask unresolved failures, guard test intact); the attempt jobs lookup read one page while full-scope runs sit near the 100-job cap (now pages until the gate job is visible); and same-attempt completion between snapshot and re-read discarded valid gate evidence (now only an advanced attempt or non-success completion rejects). * fix(pr): only newer gate-proven reruns supersede a terminal failure Autoreview P1: without an ordering bound, a stalled older run's passed gate could mask a newer completed failure. The supersede now requires the gate-proven scheduled run to be created after the failed run; regression test covers the stalled-older-run case. --- scripts/verify-pr-hosted-gates.mjs | 72 +++++++++++++++++---- test/scripts/verify-pr-hosted-gates.test.ts | 50 ++++++++++++++ 2 files changed, 109 insertions(+), 13 deletions(-) diff --git a/scripts/verify-pr-hosted-gates.mjs b/scripts/verify-pr-hosted-gates.mjs index 27d57e411d96..a414041757fc 100644 --- a/scripts/verify-pr-hosted-gates.mjs +++ b/scripts/verify-pr-hosted-gates.mjs @@ -226,8 +226,35 @@ function successfulRunOrThrow( if (isSuccessfulRecentRun(run, nowMs)) { return run; } - if (workflowName === "CI" && isGateProvenInProgressRun(run, ciGateJobs, nowMs)) { - return run; + if (workflowName === "CI") { + if (isGateProvenInProgressRun(run, ciGateJobs, nowMs)) { + return run; + } + // A terminal non-success stays blocking unless a NEWER pending SCHEDULED + // rerun on the same head has already passed its own gate — the gate needs + // every selected lane, so that attempt is authoritative proof the failure + // is re-resolved. The newer-than bound stops a stalled older run's gate + // from masking a later failure, and manual runs can never mask one. + if (run?.status === "completed" && run.conclusion !== "success") { + const failedRunCreatedAtMs = Date.parse(String(run?.created_at ?? "")); + const gateProvenRerun = matchingRuns.find((candidate) => { + if (candidate === run || candidate.event !== "pull_request") { + return false; + } + const candidateCreatedAtMs = Date.parse(String(candidate?.created_at ?? "")); + if ( + !Number.isFinite(candidateCreatedAtMs) || + !Number.isFinite(failedRunCreatedAtMs) || + candidateCreatedAtMs <= failedRunCreatedAtMs + ) { + return false; + } + return isGateProvenInProgressRun(candidate, ciGateJobs, nowMs); + }); + if (gateProvenRerun) { + return gateProvenRerun; + } + } } throw new Error( `Missing successful recent ${workflowName} workflow for ${sha}. Observed: ${formatObservedRuns(matchingRuns)}`, @@ -541,25 +568,44 @@ function loadCiGateJobs(repo, workflowRuns, sha, nowMs = Date.now()) { ); return candidates.flatMap((run) => { const attempt = run.run_attempt ?? 1; - const payload = JSON.parse( - execGhApiRead(`repos/${repo}/actions/runs/${run.id}/attempts/${attempt}/jobs?per_page=100`, { - encoding: "utf8", - stdio: ["ignore", "pipe", "pipe"], - }), - ); - const jobs = Array.isArray(payload?.jobs) ? payload.jobs : []; + // The jobs endpoint pages at 100 and full-scope runs already sit near + // that; page until the gate job is visible so growth past one page can + // never silently disable the early-proof path. + const jobs = []; + for (let page = 1; page <= 5; page += 1) { + const payload = JSON.parse( + execGhApiRead( + `repos/${repo}/actions/runs/${run.id}/attempts/${attempt}/jobs?per_page=100&page=${page}`, + { encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] }, + ), + ); + const pageJobs = Array.isArray(payload?.jobs) ? payload.jobs : []; + jobs.push(...pageJobs); + const totalCount = Number(payload?.total_count ?? 0); + if ( + pageJobs.length === 0 || + jobs.length >= totalCount || + jobs.some((job) => job?.name === CI_GATE_CHECK_NAME) + ) { + break; + } + } // Re-read the run after fetching its attempt jobs and drop the evidence if - // the attempt advanced or the run completed in between: otherwise a rerun - // starting in that window would let the just-fetched prior-attempt gate - // vouch for an attempt that has not reached its own gate. + // the attempt advanced in between: otherwise a rerun starting in that + // window would let the just-fetched prior-attempt gate vouch for an + // attempt that has not reached its own gate. Same-attempt completion is + // fine — a run that finished successfully still proves this attempt, and + // a non-success completion must not be blessed by its own earlier gate. const current = JSON.parse( execGhApiRead(`repos/${repo}/actions/runs/${run.id}`, { encoding: "utf8", stdio: ["ignore", "pipe", "pipe"], }), ); + const sameAttempt = (current?.run_attempt ?? attempt) === attempt; const stillPending = current?.status === "in_progress" || current?.status === "queued"; - if (!stillPending || (current?.run_attempt ?? attempt) !== attempt) { + const completedSuccess = current?.status === "completed" && current?.conclusion === "success"; + if (!sameAttempt || (!stillPending && !completedSuccess)) { return []; } return jobs; diff --git a/test/scripts/verify-pr-hosted-gates.test.ts b/test/scripts/verify-pr-hosted-gates.test.ts index 7caac4b93006..3588b6760cc0 100644 --- a/test/scripts/verify-pr-hosted-gates.test.ts +++ b/test/scripts/verify-pr-hosted-gates.test.ts @@ -129,6 +129,56 @@ describe("verify-pr-hosted-gates", () => { ).toThrow(/Missing successful recent CI workflow/); }); + it("lets a gate-proven pending rerun win over an older terminal failure", () => { + const failedRun = { + ...successfulRun("CI", 40, "2026-06-17T10:40:00Z"), + conclusion: "failure", + }; + const pendingRerun = { + ...successfulRun("CI", 42, "2026-06-17T10:52:00Z"), + status: "in_progress", + conclusion: null, + run_attempt: 1, + created_at: "2026-06-17T10:50:00Z", + }; + const gateJob = { + name: "openclaw/ci-gate", + run_id: 42, + run_attempt: 1, + status: "completed", + conclusion: "success", + completed_at: "2026-06-17T10:51:30Z", + }; + + // The newer pending run is re-resolving the failure; its successful gate + // proves the selected lanes, so the stale failure must not block. + const evidence = collectHostedGateEvidence({ + sha, + workflowRuns: [failedRun, pendingRerun], + ciGateJobs: [gateJob], + }); + expect(evidence.workflows.map((workflow: { id: unknown }) => workflow.id)).toContain(42); + + // Without gate proof the pending run still blocks (no early acceptance), + // and a failure that IS the latest scheduled run still blocks outright. + expect(() => + collectHostedGateEvidence({ sha, workflowRuns: [failedRun, pendingRerun], ciGateJobs: [] }), + ).toThrow(/Missing successful recent CI workflow/); + expect(() => + collectHostedGateEvidence({ sha, workflowRuns: [failedRun], ciGateJobs: [gateJob] }), + ).toThrow(/Missing successful recent CI workflow/); + + // A stalled OLDER run's gate must not mask a newer terminal failure. + const stalledOlderRun = { ...pendingRerun, created_at: "2026-06-17T10:40:00Z" }; + expect(() => + collectHostedGateEvidence({ + sha, + workflowRuns: [failedRun, stalledOlderRun], + ciGateJobs: [gateJob], + }), + ).toThrow(/Missing successful recent CI workflow/); + }); + it("requires the latest scheduled workflow run to pass", () => { const evidence = collectHostedGateEvidence({ sha,