From 79b67496699fcd31d57a951dfc432bd6e6d91718 Mon Sep 17 00:00:00 2001 From: Liangsheng Yin Date: Mon, 18 May 2026 04:23:16 -0700 Subject: [PATCH] ci: pr-states no longer overwrites running run state when label is removed (#25586) --- .github/workflows/pr-states.yml | 100 +++++++++++++++++++++----------- 1 file changed, 65 insertions(+), 35 deletions(-) diff --git a/.github/workflows/pr-states.yml b/.github/workflows/pr-states.yml index ea106fc6a..b6c785eed 100644 --- a/.github/workflows/pr-states.yml +++ b/.github/workflows/pr-states.yml @@ -1,16 +1,15 @@ name: PR States -# Maintains a CI-states block at the bottom of the PR body. Triggered by -# `pull_request_target` (not `pull_request`) so fork PRs get a write-enabled -# GITHUB_TOKEN. Safe because the workflow never checks out PR head code — -# only reads metadata via API and PATCHes the PR body. +# Maintains the CI-states block at the bottom of the PR body. Uses +# pull_request_target (not pull_request) for fork-PR write access; safe +# because we never check out PR head code, only API-read and PATCH the body. on: pull_request_target: types: [opened, synchronize, reopened, labeled, unlabeled] workflow_run: # Listen to pr-test* lifecycle so reruns initiated by slash commands - # (run.rerun / run.rerun_failed_jobs via GITHUB_TOKEN) — which don't - # refire pull_request_target — still refresh the CI-states block. + # (run.rerun / run.rerun_failed_jobs via GITHUB_TOKEN) -- which don't + # refire pull_request_target -- still refresh the CI-states block. workflows: ["PR Test", "PR Test Extra"] types: [requested, completed] @@ -33,14 +32,18 @@ jobs: uses: actions/github-script@v7 with: script: | + const RETRY_ATTEMPTS = 3; + const RETRY_DELAY_MS = 6000; + const LABEL_ON_PRESLEEP_MS = 3000; + const LABEL_OFF_PRESLEEP_MS = 6000; + const sleep = (ms) => new Promise(r => setTimeout(r, ms)); + // Event-agnostic PR lookup. // - pull_request_target: PR is in the payload. // - workflow_run: pull_requests[] is populated for same-repo PRs; - // for fork PRs it's empty and we reverse-lookup by head ref - // ("forkOwner:branch"), which is the GH-documented fork-PR query - // pattern and avoids the "commit must be in default branch" caveat - // of listPullRequestsAssociatedWithCommit. Bail cleanly when no - // open PR matches (e.g. workflow run from a push to main). + // fork PRs need a head-ref reverse lookup ("forkOwner:branch") + // because listPullRequestsAssociatedWithCommit requires the commit + // to be in the default branch. Bail when no open PR matches. let prNumber; if (context.payload.pull_request) { prNumber = context.payload.pull_request.number; @@ -79,7 +82,11 @@ jobs: const hasCI = labels.includes('run-ci'); const hasExtra = labels.includes('run-ci-extra'); - // Retry briefly: pr-test* may not be API-visible yet when we start. + // Pre-sleep below only matters when a label was just removed; + // other no-label fires don't race against fresh indexing. + const justRemovedLabel = context.eventName === 'pull_request_target' + && context.payload.action === 'unlabeled'; + async function findRunBySha(workflowFile) { const { data } = await github.rest.actions.listWorkflowRuns({ owner: context.repo.owner, @@ -90,38 +97,61 @@ jobs: }); return data.workflow_runs[0] || null; } - async function findRunWithRetry(workflowFile, maxAttempts = 6, delayMs = 5000) { - for (let i = 0; i < maxAttempts; i++) { + async function findRun(workflowFile, { preSleepMs = 0, attempts = 1, delayMs = 0 }) { + if (preSleepMs > 0) await sleep(preSleepMs); + for (let i = 0; i < attempts; i++) { const run = await findRunBySha(workflowFile); if (run) return run; - if (i < maxAttempts - 1) await new Promise(r => setTimeout(r, delayMs)); + if (i < attempts - 1) await sleep(delayMs); } return null; } - const ptRun = hasCI ? await findRunWithRetry('pr-test.yml') : null; - // pr-test-extra's gate requires BOTH run-ci AND run-ci-extra - // (see pr-test-extra.yml check-changes if-condition), so without - // run-ci the extra workflow doesn't run either. - const peRun = (hasCI && hasExtra) ? await findRunWithRetry('pr-test-extra.yml') : null; + // Query unconditionally: removing a label does NOT cancel an + // in-flight workflow, so a run from before removal can still be + // live. Label state only picks the fallback text. labelOnOpts + // covers fresh-push indexing race; labelOffOpts only pre-sleeps + // on just-removed-label to catch removal-after-push. + const labelOnOpts = { preSleepMs: LABEL_ON_PRESLEEP_MS, attempts: RETRY_ATTEMPTS, delayMs: RETRY_DELAY_MS }; + const labelOffOpts = { preSleepMs: justRemovedLabel ? LABEL_OFF_PRESLEEP_MS : 0, attempts: 1 }; + const ptRun = await findRun('pr-test.yml', hasCI ? labelOnOpts : labelOffOpts); + // pr-test-extra gates on BOTH labels (see check-changes there). + const peRun = await findRun('pr-test-extra.yml', (hasCI && hasExtra) ? labelOnOpts : labelOffOpts); - // Treat a fully-skipped run as "no real run" — happens when the PR - // was opened without the label and label was added later (GHA does - // not retrigger on `labeled`). + // Skipped run = "no real run" -- happens when a label is added + // after a commit, because GHA doesn't retrigger on `labeled`. const isReal = (run) => run && run.conclusion !== 'skipped'; - const missingCIText = ':x: **Missing `run-ci` label** — add it to run CI tests.'; - const peBlockedByCIText = ':x: **Blocked** — `run-ci` is required first.'; - const notExtraEnabledText = ':warning: **Not enabled** — add `run-ci-extra` label to opt in.'; - const stalePushText = ':warning: **Not run on latest push** — push again to dispatch.'; - const ptText = !hasCI - ? missingCIText - : (isReal(ptRun) ? `[Run #${ptRun.id}](${ptRun.html_url})` : '_Not run yet_'); - const peText = !hasCI - ? peBlockedByCIText - : !hasExtra - ? notExtraEnabledText - : (isReal(peRun) ? `[Run #${peRun.id}](${peRun.html_url})` : stalePushText); + const missingCIText = ':x: **Missing `run-ci` label** -- add it to run CI tests.'; + const peBlockedByCIText = ':x: **Blocked** -- `run-ci` is required first.'; + const notExtraEnabledText = ':warning: **Not enabled** -- add `run-ci-extra` label to opt in.'; + const stalePushText = ':warning: **Not run on latest push** -- push again to dispatch.'; + + // Status icon: body otherwise looks uniformly "dispatched" even + // for gate-failed runs (call-gate exits on missing label). + function runIcon(run) { + if (run.status !== 'completed') return ':hourglass_flowing_sand:'; + switch (run.conclusion) { + case 'success': return ':white_check_mark:'; + case 'failure': return ':x:'; + case 'cancelled': return ':no_entry_sign:'; + case 'timed_out': return ':alarm_clock:'; + case 'action_required': return ':warning:'; + case 'neutral': return ':grey_question:'; + default: return ':grey_question:'; + } + } + const runLink = (run) => `${runIcon(run)} [Run #${run.id}](${run.html_url})`; + const ptText = isReal(ptRun) + ? runLink(ptRun) + : (!hasCI ? missingCIText : '_Not run yet_'); + const peText = isReal(peRun) + ? runLink(peRun) + : !hasCI + ? peBlockedByCIText + : !hasExtra + ? notExtraEnabledText + : stalePushText; const outerStart = ''; const outerEnd = '';