mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-02 11:51:35 +00:00
fix(pr): close the review follow-ups on early gate proof (#109513)
* 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.
This commit is contained in:
committed by
GitHub
parent
9bd51e97fa
commit
aafbf7aabc
@@ -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;
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user