From 2d50a25436a8c9a77d66dec823164cb777d826af Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Wed, 4 Nov 2026 08:11:11 -0500 Subject: [PATCH] fix(ci): re-send an activity a head-lag stop lost, once check-trusted-trigger stops a run head-lag when GET /pulls and the branch ref never agree inside its 60 s wait, and report-check-run posts that as a neutral superseded-sha skip on the head the select job read. The run ends green, so the tick's workflow-modified refire (sweepRefused) never took it, and the skip handled the head for every other pass: #13194's address-feedback run 41826593020 needed a hand re-dispatch. sweepRefused now also takes the PR's newest address-feedback run when its own check on the PR's current head is that skip. Every other superseded-sha check lands on a head the PR has left, so a head that moved since keeps its stop. The same rules hold as for a refusal: newest run only, inside FAILED_RUN_WINDOW_HOURS, under SWEEP_HEAD_CAP, earlier checks released through requestSince, and the record names the stopped run as refire_of_run_id. Co-Authored-By: Claude Opus 5.5 --- .../actions/check-trusted-trigger/README.md | 4 +- .../scripts/pr-unblock/feedback-dispatch.mjs | 56 +++++++++------ .../pr-unblock/feedback-dispatch.test.mjs | 68 +++++++++++++++++++ 3 files changed, 108 insertions(+), 20 deletions(-) diff --git a/.github/actions/check-trusted-trigger/README.md b/.github/actions/check-trusted-trigger/README.md index b3095e601a8..b86a4bae82c 100644 --- a/.github/actions/check-trusted-trigger/README.md +++ b/.github/actions/check-trusted-trigger/README.md @@ -44,7 +44,9 @@ condition 1 and the actor and author parts of condition 2). after 5, 10, 20 and 25 s until its `head.sha` equals its branch ref; if they never agree, or the branch ref is missing, the lane stops with `head-lag` rather than gate a stale head, which `report-check-run` posts as a `superseded-sha` skip, since the push that moved the branch is - gated by its own run; a failed read stops with `no-pr`. A fork's ref is not read. + gated by its own run. When that skip lands on the PR's current head instead, the + address-feedback tick sends the round once more (`feedback-dispatch.mjs` `sweepRefused`); a + failed read stops with `no-pr`. A fork's ref is not read. - **Dispatcher-sent `workflow_dispatch`.** A dispatch made with `GITHUB_TOKEN` names `github-actions[bot]` (id 41898282) as sender, actor and triggering actor, and nothing on the run names who started the dispatcher. When `sender.id` is 41898282 and `dispatcher-run-id` is diff --git a/.github/scripts/pr-unblock/feedback-dispatch.mjs b/.github/scripts/pr-unblock/feedback-dispatch.mjs index 50d69ac8c3e..b09e09cf8fd 100644 --- a/.github/scripts/pr-unblock/feedback-dispatch.mjs +++ b/.github/scripts/pr-unblock/feedback-dispatch.mjs @@ -138,10 +138,12 @@ // head answered "no earlier FAIL" may be read again inside that window. // // Before every other sweep, the tick sends again a round the token broker -// refused as workflow-modified, once per refused run (sweepRefused): a round -// in flight when a lane workflow file changes on main is refused its push or -// reply token, a re-run keeps the old file, and the head's check runs would -// otherwise hold it. +// refused as workflow-modified, or the gate stopped head-lag, once per stopped +// run (sweepRefused): a round in flight when a lane workflow file changes on +// main is refused its push or reply token, and a re-run keeps the old file; a +// round whose gate could not match the PR's head to its branch ref within its +// wait (an API lag after an outage) did no work. Either way the head's check +// runs would otherwise hold it. // // A run of this file an outage lost, or whose dispatch job failed, dispatched // nothing the lanes trust when re-run: check-trusted-trigger's dispatcher @@ -261,6 +263,12 @@ const startOf = (run) => Date.parse(run.started_at ?? ""); // and so is a kill-switch stop, which posted no reply. const RELEASES = new Set(["Skipped: no-model-turn", "Skipped: finding-stands", LANE_STOP, KILL_SWITCH_STOP]); const releases = (run) => run?.status === "completed" && run.conclusion === "neutral" && RELEASES.has(run.output?.title); +// The gate's head-lag stop (check-trusted-trigger.mjs fetchPull), which +// report-check-run posts as a superseded-sha skip (gate-skip-reasons.json) on +// the head the select job read. Every other superseded-sha check lands on a +// head the PR has left (a cancel, apply-patch's moved tip), so one on the +// PR's current head is a lag stop that did no work there. +const headLagStop = (run) => run.status === "completed" && run.conclusion === "neutral" && run.output?.title === "Skipped: superseded-sha"; // A pass-only round's reply check (publish-feedback.mjs PASS_ONLY_SKIP) // answers no thread, so it neither releases a head nor ends a release. const roundReplies = (runs) => runs.filter((run) => run.output?.title !== "Skipped: pass-only"); @@ -846,7 +854,7 @@ export const SWEEP_TICK_CAP = 5; export const SWEEP_DECIDE_BUDGET_MS = 5 * 60_000; // The sweep, in pass order: each open PR whose newest round the token broker -// refused as workflow-modified (sweepRefused); each open lanes App PR into +// refused as workflow-modified or the gate stopped head-lag (sweepRefused); each open lanes App PR into // main with a fix request newer than its head's last round (sweepFixRequest); each whose // head's newest reply check took a releasing skip, under SWEEP_HEAD_CAP such // checks (sweepOne); then sweepStale, sweepUnsent and sweepIdleOwner. Each PR @@ -865,7 +873,7 @@ export async function sweepReleased({ api, repository, appSenderId, log, setting const taken = new Set(); const started = clock(); const passes = [ - [sweepRefused, listed, "sweep: no round the broker refused as workflow-modified to send again"], + [sweepRefused, listed, "sweep: no round the broker refused as workflow-modified or the gate stopped head-lag to send again"], [sweepFixRequest, pulls, "sweep: no head with a fix request newer than its last round"], [sweepOne, pulls, "sweep: no head whose last round left it for another round to dispatch"], [sweepStale, pulls, "sweep: no head with a failing address-feedback stop and nothing left to fix"], @@ -1199,8 +1207,11 @@ async function dispatchAtHead({ api, pull, repository, appSenderId, log, setting // SWEEP_HEAD_CAP address-feedback check runs, which bounds a chain of // refusals. Check runs started before the refused run ended stop counting // (requestSince), but a limit stop keeps its reset hold. A refusal for any -// other reason is left alone. The result names the refused run (refireOf), -// which the dispatched-run record keeps. +// other reason is left alone. A run the gate stopped head-lag counts the same +// way, whatever its conclusion, when its own address-feedback check on the +// PR's current head is that stop (headLagStop): a head that moved since was +// gated by its own run, so the stop was right there. The result names the +// stopped run (refireOf), which the dispatched-run record keeps. async function sweepRefused(context) { const { api, pull, log, settings, rest } = context; const head = pull.head.sha; @@ -1208,23 +1219,30 @@ async function sweepRefused(context) { const [last] = (await api.listWorkflowRuns(settings.workflow)) .filter((run) => run.display_title === runTitle(settings.activity, pull.number)) .sort((a, b) => Date.parse(b.created_at ?? "") - Date.parse(a.created_at ?? "")); - if (last?.status !== "completed" || last.conclusion !== "failure" || !RUN_ID.test(String(last.id)) || !(Date.parse(last.updated_at ?? "") >= since)) { + if (last?.status !== "completed" || !RUN_ID.test(String(last.id)) || !(Date.parse(last.updated_at ?? "") >= since)) { return null; } const rounds = await api.listCheckRuns({ sha: head, checkName: settings.checkName, appId: settings.appId }); - const posted = (checks) => checks.some((check) => postingRunId(check) === String(last.id)); - if (!posted(rounds) && !posted(await api.listCheckRuns({ sha: head, checkName: settings.replyCheckName, appId: settings.appId }))) { - return null; - } - const why = await tokenRefusal({ jobs: await api.listRunJobs(last.id), readAnnotations: api.listAnnotations, activity: settings.activity }); - if (why?.kind !== "workflow-modified") { - return null; + const own = (checks) => checks.filter((check) => postingRunId(check) === String(last.id)); + let stop = own(rounds).some(headLagStop) ? "stopped head-lag" : ""; + if (!stop) { + if (last.conclusion !== "failure") { + return null; + } + if (own(rounds).length === 0 && own(await api.listCheckRuns({ sha: head, checkName: settings.replyCheckName, appId: settings.appId })).length === 0) { + return null; + } + const why = await tokenRefusal({ jobs: await api.listRunJobs(last.id), readAnnotations: api.listAnnotations, activity: settings.activity }); + if (why?.kind !== "workflow-modified") { + return null; + } + stop = `refused as workflow-modified (${why.path})`; } if (rounds.length >= SWEEP_HEAD_CAP) { - log(`skip: #${pull.number}: run ${last.id} was refused as workflow-modified (${why.path}), and head ${head} has ${rounds.length} rounds, the sweep's cap`); + log(`skip: #${pull.number}: run ${last.id} was ${stop}, and head ${head} has ${rounds.length} rounds, the sweep's cap`); return null; } - log(`refire: #${pull.number}: run ${last.id} was refused as workflow-modified (${why.path}) at head ${head}; sending ${settings.activity} again on main`); + log(`refire: #${pull.number}: run ${last.id} was ${stop} at head ${head}; sending ${settings.activity} again on main`); const dispatched = await dispatchAtHead(context, { requestSince: Date.parse(last.updated_at) }); return dispatched === null ? null : { ...dispatched, refireOf: String(last.id) }; } @@ -1730,7 +1748,7 @@ export async function main(env, { api, github, resolveSlot, verifyCapped, log = // Each send's outputs are written as it is sent, so a later send's // failure still leaves the earlier ones recorded: the first as run-id, // the nth as run-id-. A round sent again after a workflow-modified - // refusal also names the refused run, as refire-of-. + // refusal or a head-lag stop also names the stopped run, as refire-of-. let sends = 0; const write = (swept) => { sends += 1; diff --git a/.github/scripts/pr-unblock/feedback-dispatch.test.mjs b/.github/scripts/pr-unblock/feedback-dispatch.test.mjs index 89ecfadd13b..cd484c69285 100644 --- a/.github/scripts/pr-unblock/feedback-dispatch.test.mjs +++ b/.github/scripts/pr-unblock/feedback-dispatch.test.mjs @@ -2481,6 +2481,74 @@ test("a round refused for another reason, outside the window, on another head or } }); +// A round whose gate could not match the PR's head to its branch ref within +// its wait (check-trusted-trigger.mjs head-lag): the run ends green, and its +// address-feedback and publish-feedback checks are superseded-sha skips on the +// head the select job read (run 41826593020 on #13194). +const LAGGED_RUN = 710; +const laggedRun = (overrides = {}) => refusedRun({ id: LAGGED_RUN, conclusion: "success", ...overrides }); +const laggedChecks = (run = LAGGED_RUN) => + ["address-feedback", "publish-feedback"].map((name) => ({ + ...roundRun(name, "neutral", "Skipped: superseded-sha", "2026-10-17T13:05:00Z"), + external_id: `pr-unblock:${name}:${run}:1`, + })); +const laggedApi = ({ runs = [laggedRun()], checksAt = { [HEAD]: laggedChecks() } } = {}) => { + const api = refusedApi({ runs, checksAt }); + api.listRunJobs = async () => assert.fail("a head-lag stop is read from its check, not its jobs"); + return api; +}; + +test("a tick sends a round the gate stopped head-lag at the PR's current head again once, and the record names the stopped run", async (t) => { + const api = laggedApi(); + const { output, lines } = await sweepMain(t, api, TICK, NOW); + assert.deepEqual(api.calls.dispatch, [ + { workflow: "pr-unblock.yml", ref: "refs/heads/main", inputs: { "pr-number": "42", activity: "address-feedback", "dispatcher-run-id": RUN_ID } }, + ]); + assert.equal(output, `run-id=38779200001\nupstream-run-id=13\nrefire-of=${LAGGED_RUN}\n`); + assert.ok(lines.some((line) => line.startsWith(`refire: #42: run ${LAGGED_RUN} was stopped head-lag at head ${HEAD}`)), lines.join("\n")); +}); + +test("a head-lag stop is not sent again once a newer run exists, the refire's included", async (t) => { + const later = { id: 711, created_at: "2026-10-17T13:45:00Z", updated_at: "2026-10-17T13:50:00Z" }; + const passed = { ...roundRun("address-feedback", "success", "Passed", "2026-10-17T13:46:00Z"), external_id: "pr-unblock:address-feedback:711:1" }; + for (const [label, run, checks] of [ + ["a newer run is still going", laggedRun({ ...later, status: "in_progress", conclusion: null }), laggedChecks()], + ["a newer run passed", laggedRun(later), [...laggedChecks(), passed]], + ["a newer run failed for another reason", laggedRun({ ...later, conclusion: "failure" }), [...laggedChecks(), { ...passed, conclusion: "failure", output: { title: "Failed", summary: "" } }]], + ]) { + const api = laggedApi({ runs: [run, laggedRun()], checksAt: { [HEAD]: checks } }); + api.listRunJobs = async () => []; + const { output } = await sweepMain(t, api, TICK, NOW); + assert.deepEqual(api.calls.dispatch, [], label); + assert.equal(output, "", label); + } +}); + +test("a head-lag stop on a head the PR has left, outside the window or at the cap is not sent again", async (t) => { + for (const [label, options] of [ + ["the head moved since", { checksAt: { [SECOND_HEAD]: laggedChecks() } }], + ["stopped hours ago", { runs: [laggedRun({ updated_at: "2026-10-17T11:00:00Z" })] }], + ["the head is at the cap", { checksAt: { [HEAD]: [...laggedChecks(), answeredRound, answeredRound] } }], + ["another run's stop", { checksAt: { [HEAD]: laggedChecks(699) } }], + ]) { + const api = laggedApi(options); + const { output } = await sweepMain(t, api, TICK, NOW); + assert.deepEqual(api.calls.dispatch, [], label); + assert.equal(output, "", label); + } +}); + +test("the head-lag refire is sent once: the round it starts is the PR's newest run", async (t) => { + const api = laggedApi(); + await sweepMain(t, api, TICK, NOW); + assert.equal(api.calls.dispatch.length, 1); + const refire = laggedRun({ id: 38779200001, status: "queued", conclusion: null, created_at: "2026-10-17T14:00:01Z", updated_at: "2026-10-17T14:00:01Z" }); + const next = laggedApi({ runs: [refire, laggedRun()] }); + const { output } = await sweepMain(t, next, TICK, NOW + 30 * 60_000); + assert.deepEqual(next.calls.dispatch, []); + assert.equal(output, ""); +}); + // A verdict the kill switch deferred: the reconciler re-ran post-verify-notes // (attempt 2, github-actions[bot]), which posted the notes or found the stale // ones and uploaded pr-feedback-deferred--, since the gate