diff --git a/.changes/pr-pipeline/fix-kill-switch-strand-pr-unblock-5c2e8a17.md b/.changes/pr-pipeline/fix-kill-switch-strand-pr-unblock-5c2e8a17.md new file mode 100644 index 0000000000..7d3e9c7150 --- /dev/null +++ b/.changes/pr-pipeline/fix-kill-switch-strand-pr-unblock-5c2e8a17.md @@ -0,0 +1,28 @@ +--- +bump: minor +--- + +### Changed + +- A new `skip-reason`, `kill-switch`: a lanes kill switch stopped the run. Every lane's gate stop by + its own kill-switch step now posts neutral `kill-switch` instead of `prerequisite-missing` + (`report-check-run`'s `gate-skip-reasons.json`), in every lane check it posts. + +### Fixed + +- A pr-unblock or pr-refine write job that the token broker refuses with reason `kill-switch` (a + switch turned on after the run's own kill-switch step read it) no longer ends red. + `pr-run-activity-write.yml` now passes `kill-switch-ok` on a pull request run, so the token step + ends green with no token, every later step skips, and the check is neutral `kill-switch`. An + issue run (work-resolve-issue) stays red, since work-unblock re-fires its issue from the failed + mint step, and so does pr-unblock's `report-escalation`, which nothing re-fires. Before, a reply job stopped this way ended `feedback-publish-failed`, routed to a + failing status. + +- The pr-unblock dispatchers dispatch a head again once the kill switch lifts. A `kill-switch` + check never handles a head (`limit-stop.mjs` `killSwitchStop`), and on a push, reply or verdict + job's check it releases the head's earlier activity checks as a lane stop does: in + `stopRelease` (resolve-conflicts), the address-feedback reply releases and half-hourly sweep, + the review-dependency verdict release, and fix-ci, where it spends no restart. Such stops count + toward `limits.limit-stops-per-head` (`SWEEP_HEAD_CAP` for address-feedback), so a switch that + keeps flipping mid-run still ends the head. Before, a head whose activity check had run stayed + handled until a new push. diff --git a/.github/actions/report-check-run/decide.test.mjs b/.github/actions/report-check-run/decide.test.mjs index c57b1c805b..0690590505 100644 --- a/.github/actions/report-check-run/decide.test.mjs +++ b/.github/actions/report-check-run/decide.test.mjs @@ -19,9 +19,10 @@ const SKIP_REASONS = [ "superseded-sha", "disabled-by-config", "untrusted-trigger", + "kill-switch", ]; const GATE_SKIP_REASONS = { - "kill-switch": "prerequisite-missing", + "kill-switch": "kill-switch", "list-unreadable": "prerequisite-missing", fork: "untrusted-trigger", "no-pr": "untrusted-trigger", @@ -257,7 +258,7 @@ test("an empty env head SHA with a reason other than no-pr posts on the fallback test("each gate reason maps to its contract skip reason", () => { const expected = [ - ["kill-switch", "kill-switch", "prerequisite-missing"], + ["kill-switch", "kill-switch", "kill-switch"], ["trigger", "list-unreadable", "prerequisite-missing"], ["trigger", "fork", "untrusted-trigger"], ["trigger", "bot-actor", "untrusted-trigger"], @@ -540,6 +541,15 @@ test("every key of the shipped gate-skip-reasons.json posts neutral with its map } }); +// request-lane-token's kill-switch-ok: the broker refused the mint for a +// switch turned on after the kill step proceeded, so no gate stopped the run +// and the token step's skip reason alone names it. +test("a broker kill-switch refusal after the gates proceeded posts neutral kill-switch", () => { + const report = posted(decide(verdict({ "skip-reason": "kill-switch" }), inputs({ actOutcome: "skipped" }))); + assert.equal(report.conclusion, "neutral"); + assert.equal(report.output.title, "Skipped: kill-switch"); +}); + test("a skip-reason outside the enum posts failure", () => { for (const reason of ["looks-fine", ""]) { const report = posted(decide(verdict({ "skip-reason": reason }), inputs())); diff --git a/.github/actions/report-check-run/gate-skip-reasons.json b/.github/actions/report-check-run/gate-skip-reasons.json index 3dc27159e5..9fdc55207d 100644 --- a/.github/actions/report-check-run/gate-skip-reasons.json +++ b/.github/actions/report-check-run/gate-skip-reasons.json @@ -1,5 +1,5 @@ { - "kill-switch": "prerequisite-missing", + "kill-switch": "kill-switch", "list-unreadable": "prerequisite-missing", "fork": "untrusted-trigger", "no-pr": "untrusted-trigger", diff --git a/.github/actions/resolve-config/callers.test.mjs b/.github/actions/resolve-config/callers.test.mjs index 29c52c4e97..b3d0efe971 100644 --- a/.github/actions/resolve-config/callers.test.mjs +++ b/.github/actions/resolve-config/callers.test.mjs @@ -6907,14 +6907,29 @@ test("every model leg is followed by a sealed transcript upload of its own execu // token, so nothing after it may run on success() alone: every later step is // gated, a step that reads the token runs only when it is set, and a job that // needs the stopped job's success and reads none of its outputs also needs its -// skip output empty (an output only a token step sets already stops it). +// skip output empty (an output only a token step sets already stops it). A +// reusable workflow's caller job stands for its job, so the same holds for +// the caller's dependents, through the skip output the workflow exposes. test("every kill-switch-ok caller runs no token step and no dependent job on a kill-switch stop", () => { const callers = []; - for (const name of readdirSync(new URL("../../workflows/", import.meta.url)).filter((f) => /\.ya?ml$/.test(f))) { - const { jobs } = workflow(name); + const dependents = (name, jobs, id, skip) => { + for (const [otherId, other] of Object.entries(jobs)) { + const needs = [other.needs ?? []].flat(); + const cond = String(other.if ?? ""); + if (!needs.includes(id) || !cond.includes(`needs.${id}.result == 'success'`)) continue; + if (cond.replaceAll(`needs.${id}.outputs.${skip} `, "").includes(`needs.${id}.outputs.`)) continue; + assert.ok(skip, `${name}:${id} exposes the token step's skip-reason for ${otherId}`); + assert.ok(cond.includes(`needs.${id}.outputs.${skip} == ''`), `${name}:${otherId} skips on ${id}'s ${skip}`); + } + }; + const names = readdirSync(new URL("../../workflows/", import.meta.url)).filter((f) => /\.ya?ml$/.test(f)); + for (const name of names) { + const parsed = workflow(name); + const { jobs } = parsed; for (const [id, job] of Object.entries(jobs ?? {})) { const steps = job.steps ?? []; - const at = steps.findIndex((step) => step.with?.["kill-switch-ok"] === true); + // A literal true, or an expression that may render it. + const at = steps.findIndex((step) => step.with?.["kill-switch-ok"] === true || String(step.with?.["kill-switch-ok"] ?? "").includes("${{")); if (at === -1) continue; callers.push(`${name}:${id}`); const tokenId = steps[at].id; @@ -6928,13 +6943,17 @@ test("every kill-switch-ok caller runs no token step and no dependent job on a k } } const [skip] = Object.entries(job.outputs ?? {}).find(([, value]) => value.includes(`steps.${tokenId}.outputs.skip-reason`)) ?? []; - for (const [otherId, other] of Object.entries(jobs)) { - const needs = [other.needs ?? []].flat(); - const cond = String(other.if ?? ""); - if (!needs.includes(id) || !cond.includes(`needs.${id}.result == 'success'`)) continue; - if (cond.replaceAll(`needs.${id}.outputs.${skip} `, "").includes(`needs.${id}.outputs.`)) continue; - assert.ok(skip, `${name}:${id} exposes the token step's skip-reason for ${otherId}`); - assert.ok(cond.includes(`needs.${id}.outputs.${skip} == ''`), `${name}:${otherId} skips on ${id}'s ${skip}`); + dependents(name, jobs, id, skip); + const called = parsed.on?.workflow_call; + if (!called) continue; + const [callSkip] = Object.entries(called.outputs ?? {}).find(([, output]) => String(output.value).includes(`jobs.${id}.outputs.${skip}`)) ?? []; + for (const caller of names) { + const callerJobs = workflow(caller).jobs ?? {}; + for (const [callerId, callerJob] of Object.entries(callerJobs)) { + if (String(callerJob.uses ?? "").split(" ")[0] !== `./.github/workflows/${name}`) continue; + callers.push(`${caller}:${callerId} -> ${name}`); + dependents(caller, callerJobs, callerId, callSkip); + } } } } @@ -6943,5 +6962,32 @@ test("every kill-switch-ok caller runs no token step and no dependent job on a k "intake-triage.yml:apply-verdict", "pr-merge.yml:enable-auto-merge", "pr-merge.yml:merge", - ]); + "pr-refine.yml:apply-docs-patch -> pr-run-activity-write.yml", + "pr-run-activity-write.yml:run", + "pr-unblock.yml:apply-conflict-patch -> pr-run-activity-write.yml", + "pr-unblock.yml:apply-dependency-patch -> pr-run-activity-write.yml", + "pr-unblock.yml:apply-dependency-verdict -> pr-run-activity-write.yml", + "pr-unblock.yml:apply-feedback-patch -> pr-run-activity-write.yml", + "pr-unblock.yml:apply-patch -> pr-run-activity-write.yml", + "pr-unblock.yml:apply-patch-rung-up -> pr-run-activity-write.yml", + "pr-unblock.yml:publish-feedback -> pr-run-activity-write.yml", + "pr-unblock.yml:report-escalation -> pr-run-activity-write.yml", + "work-resolve-issue.yml:apply-issue-patch -> pr-run-activity-write.yml", + "work-resolve-issue.yml:report-outcome -> pr-run-activity-write.yml", + "work-resolve-issue.yml:report-skip -> pr-run-activity-write.yml", + ].sort()); +}); + +// work-unblock re-fires an issue from the failed mint step, and no +// dispatcher re-fires report-escalation, so only a pull request run of any +// other activity ends a kill-switch refusal green; those two stay red. +test("pr-run-activity-write.yml takes kill-switch-ok on a pull request run only, never for report-escalation", () => { + const step = workflow("pr-run-activity-write.yml").jobs.run.steps.find((s) => s.id === "lane-token"); + const value = step.with["kill-switch-ok"]; + const at = (issue, activity) => evaluate(value, { "inputs.issue-number": issue, "inputs.activity": activity }); + assert.equal(at("", "publish-feedback"), true, "a pull request run skips"); + assert.equal(at("7", "apply-issue-patch"), false, "an issue run stays red"); + assert.equal(at("", "report-escalation"), false, "an escalation stays red"); + const { jobs } = workflow("pr-unblock.yml"); + assert.equal(jobs["report-escalation"].with.activity, "report-escalation", "the escalation caller passes that activity"); }); diff --git a/.github/scripts/pr-unblock/dependency-dispatch.mjs b/.github/scripts/pr-unblock/dependency-dispatch.mjs index 5ccb9d270c..8e65e130d2 100644 --- a/.github/scripts/pr-unblock/dependency-dispatch.mjs +++ b/.github/scripts/pr-unblock/dependency-dispatch.mjs @@ -7,8 +7,8 @@ // apply-dependency-verdict check run skipped no-model-turn: the // review-dependency check runs that started before it handled nothing, so the // head is dispatched again. A verdict check that is a lane stop (a reason -// escalation-routes.json routes to a failing status) releases them the same -// way, until `limits.limit-stops-per-head` such stops stand on the head, as +// escalation-routes.json routes to a failing status) or a kill-switch stop +// releases them the same way, until `limits.limit-stops-per-head` such stops stand on the head, as // in dispatch.mjs. The model job's own selection decides the round // cap, and every such round posts its round marker. A limit stop holds the // head only until its limit resets, at most `limits.limit-stops-per-head` @@ -133,8 +133,9 @@ export async function dispatchDependencyReviews({ } existing = await api.listCheckRuns({ sha: node.headRefOid, checkName, appId }); // A round whose verdict job skipped no-model-turn handled nothing, and - // one whose verdict job stopped on a reason routed to a failing status - // is retried on the same head, a fresh model session, until + // one whose verdict job stopped on a reason routed to a failing status, + // or that the kill switch stopped (limit-stop.mjs releasesHead), is + // retried on the same head, a fresh model session, until // limit-stops-per-head such stops stand on it: either way the // review-dependency check runs that started before that verdict job // stop counting, so this run dispatches the head again. A limit stop is diff --git a/.github/scripts/pr-unblock/dependency-dispatch.test.mjs b/.github/scripts/pr-unblock/dependency-dispatch.test.mjs index 7b1584649a..acb3d4fea6 100644 --- a/.github/scripts/pr-unblock/dependency-dispatch.test.mjs +++ b/.github/scripts/pr-unblock/dependency-dispatch.test.mjs @@ -235,6 +235,23 @@ test("a head whose last round had no model turn dispatches again, and is not rec } }); +// A kill switch turned on after the model call ran stops the verdict job at +// the broker (request-lane-token kill-switch-ok): its check skips +// kill-switch and acts on nothing, so the head is released like a +// no-model-turn round, and so is a kill-switch stop of the review itself. +test("a head the kill switch stopped dispatches again, and is not recorded as handled", async () => { + for (const [label, checks] of [ + ["the verdict job's mint was refused", [roundRun("review-dependency", "success", "Passed", "2026-10-17T10:00:00Z"), roundRun("apply-dependency-verdict", "neutral", "Skipped: kill-switch", "2026-10-17T10:05:00Z")]], + ["the review run's own kill step stopped it", [roundRun("review-dependency", "neutral", "Skipped: kill-switch", "2026-10-17T10:00:00Z")]], + ]) { + const node = pr(60); + const record = emptyRecord(); + const { dispatched } = await run(fakeApi({ nodes: [node], checks: headOf(node, checks) }), { record }); + assert.deepEqual(dispatched, [{ number: 60, workflowRunId: "7060" }], label); + assert.equal(record.handled.get(CHECK), undefined, label); + } +}); + // A review-dependency run the usage limit stopped at `started`, its lane record // naming a reset an hour later. const limitStop = (started) => { diff --git a/.github/scripts/pr-unblock/feedback-dispatch.mjs b/.github/scripts/pr-unblock/feedback-dispatch.mjs index 1bea6534c0..d4c6327333 100644 --- a/.github/scripts/pr-unblock/feedback-dispatch.mjs +++ b/.github/scripts/pr-unblock/feedback-dispatch.mjs @@ -31,8 +31,9 @@ // - that head has no pr-unblock / address-feedback check run yet (one the // usage limit stopped stops counting once its limit resets, and one that // started before the head's newest pr-unblock / publish-feedback check run -// when that run skipped no-model-turn or finding-stands: that round left -// the head for another round), and no +// when that run skipped no-model-turn, finding-stands, lane-stopped or +// kill-switch: that round left the head for another round; a kill-switch +// stop of the address-feedback run itself does not count either), and no // pr-unblock.yml address-feedback run for the PR is still queued or going; // - the base config's pr-unblock lane is enabled, holds enabled // address-feedback and publish-feedback slots, and address-feedback applies @@ -62,11 +63,13 @@ // on a PR a person opened, verify-notes.mjs holds a fail with a label it // removes itself on the next pass, so there is no thread to resolve. // -// A round whose reply job skipped no-model-turn, or finding-stands (an +// A round whose reply job skipped no-model-turn, finding-stands (an // adjudicator let a disputed finding stand and the head did not move, so no -// review run follows), releases its head only for the next review or -// pr-verify completion there, which may never come. So a completed -// scheduled pr-unblock-dispatch.yml run, the half-hourly tick, also starts +// review run follows), or kill-switch (the lanes kill switch stopped it, and +// the switch lifting starts no review run), releases its head only for the +// next review or pr-verify completion there, which may never come. So a +// completed scheduled pr-unblock-dispatch.yml run, the half-hourly tick, which +// the kill switch also stops, starts // the file, and main() sweeps (sweepReleased): each open lanes App PR into // main whose head's newest reply check took one of those skips, fewer than // SWEEP_HEAD_CAP times at that head, is decided as above with the @@ -117,7 +120,7 @@ import { LOOKBACK_HOURS, LOST } from "../maintenance-reconcile-lost-runs/infra-f import { fixRequesters, humanGatedLabelFrom, onReviewedHead, reviewedHeads, reviewerIdsFrom, threadAuthors, trustedHumansFrom } from "./address-feedback.mjs"; import { commentItems, gateSource, listLabelEvents } from "./gate-source.mjs"; import { emptyRecord, isHandled, markHandled, readRecord, writeRecord } from "./dispatch-record.mjs"; -import { anyHandles, LANE_STOP, laneStop, limitResetsAt, permanentlyHandles } from "./limit-stop.mjs"; +import { anyHandles, KILL_SWITCH_STOP, LANE_STOP, laneStop, limitResetsAt, permanentlyHandles } from "./limit-stop.mjs"; import { budgetedFetch } from "./request-budget.mjs"; export const DEFAULTS = Object.freeze({ @@ -163,8 +166,9 @@ export const SWEEP_HEAD_CAP = 3; const startOf = (run) => Date.parse(run.started_at ?? ""); // The reply check skips that leave the head for another round: a lane stop // (a stop escalation-routes.json routes to a failing status) is retried on -// the same head, a fresh model session, under SWEEP_HEAD_CAP with the rest. -const RELEASES = new Set(["Skipped: no-model-turn", "Skipped: finding-stands", LANE_STOP]); +// the same head, a fresh model session, under SWEEP_HEAD_CAP with the rest, +// 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); // 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. diff --git a/.github/scripts/pr-unblock/feedback-dispatch.test.mjs b/.github/scripts/pr-unblock/feedback-dispatch.test.mjs index ebddfc8b6c..0df55b533f 100644 --- a/.github/scripts/pr-unblock/feedback-dispatch.test.mjs +++ b/.github/scripts/pr-unblock/feedback-dispatch.test.mjs @@ -1435,6 +1435,44 @@ test("a scheduled tick retries a head whose reply job stopped, until SWEEP_HEAD_ assert.deepEqual(terminal.calls.dispatch, []); }); +// A kill switch turned on after a round's model leg ran stops its reply job +// at the broker (request-lane-token kill-switch-ok): its publish-feedback +// check skips kill-switch and posts no reply, so the head is released like a +// lane stop. The tick, which the switch also stops, re-dispatches it once the +// switch lifts, under SWEEP_HEAD_CAP; a trigger at that head does too, and so +// does one after a kill-switch stop of the address-feedback run itself. +const killSwitched = (name, started) => roundRun(name, "neutral", "Skipped: kill-switch", started); + +test("a scheduled tick re-dispatches a head whose reply job the kill switch stopped, until SWEEP_HEAD_CAP stops stand", async (t) => { + const api = sweepApi({ checksAt: { [HEAD]: [answeredRound, killSwitched("publish-feedback", "2026-10-17T10:05:00Z")] } }); + await sweepMain(t, api); + assert.deepEqual(api.calls.dispatch.map((call) => call.inputs["pr-number"]), ["42"]); + const capped = sweepApi({ + checksAt: { + [HEAD]: [ + answeredRound, + killSwitched("publish-feedback", "2026-10-17T10:05:00Z"), + killSwitched("publish-feedback", "2026-10-17T11:05:00Z"), + killSwitched("publish-feedback", "2026-10-17T12:05:00Z"), + ], + }, + }); + assert.equal((await sweepMain(t, capped)).output, ""); + assert.deepEqual(capped.calls.dispatch, []); +}); + +test("a head the kill switch stopped dispatches again, and is not recorded as handled", async () => { + for (const [label, checks] of [ + ["the reply job's mint was refused", [roundRun("address-feedback", "success", "Passed", "2026-10-17T10:00:00Z"), killSwitched("publish-feedback", "2026-10-17T10:05:00Z")]], + ["the address-feedback run's own kill step stopped it", [killSwitched("address-feedback", "2026-10-17T10:00:00Z")]], + ]) { + const record = emptyRecord(); + const { result } = await decideWith(fakeApi({ checkRuns: checks }), record); + assert.deepEqual(result, { number: 42, workflowRunId: "38779200001" }, label); + assert.equal(record.handled.size, 0, label); + } +}); + // The recovery: a pr-feedback-dispatch.yml run an outage lost in the // lookback window (24 hours before NOW) makes the tick decide every open // lanes App head through dispatchFeedback. `runs` is the one listing, `jobs` diff --git a/.github/scripts/pr-unblock/fix-ci-dispatch.mjs b/.github/scripts/pr-unblock/fix-ci-dispatch.mjs index 1df8ef2ddd..635625b528 100644 --- a/.github/scripts/pr-unblock/fix-ci-dispatch.mjs +++ b/.github/scripts/pr-unblock/fix-ci-dispatch.mjs @@ -56,7 +56,7 @@ import { emptyRecord, isHandled, markHandled } from "./dispatch-record.mjs"; import { runTitle } from "./feedback-dispatch.mjs"; import { failingChecks } from "./fix-ci-context.mjs"; import { prHold, reviewReader } from "./gate-source.mjs"; -import { laneStop } from "./limit-stop.mjs"; +import { KILL_SWITCH_STOP, killSwitchStop, laneStop } from "./limit-stop.mjs"; import { serverFailure } from "./request-budget.mjs"; export const DEFAULTS = Object.freeze({ @@ -165,7 +165,9 @@ const fromRest = (run) => ({ // is one attempt, read from its last check, that spends one place under the // per-head cap for each of its checks (`spent`). An attempt a lane-stopped push job // check (`stopRuns`, REST) followed reads as a failure: the run stopped, so -// the head may be retried under the same caps a failed run is. A check's +// the head may be retried under the same caps a failed run is. One a +// kill-switch push job check followed reads as that kill-switch skip, which +// did no work: it is retried like a held skip, spending no restart. A check's // attempt is the workflow run attempt postingAttempt names; not its check // suite, which GitHub picks for a check created over the API. A stop marks // only its own run attempt, so a re-run is a new attempt; a check with no @@ -174,8 +176,9 @@ const fromRest = (run) => ({ export function withStops(runs, stopRuns) { const startOf = (run) => Date.parse(run.started_at ?? ""); const stops = stopRuns.filter(laneStop); + const kills = stopRuns.filter(killSwitchStop); const starts = runs.map(startOf).sort((a, b) => a - b); - const stoppedAt = (run) => { + const stoppedAt = (run, stops) => { const from = startOf(run); const until = starts.find((start) => start > from) ?? Infinity; return stops.some((stop) => @@ -187,25 +190,27 @@ export function withStops(runs, stopRuns) { const attempts = new Map(); for (const run of runs) { const key = postingAttempt(run) ?? Symbol(); - const attempt = attempts.get(key) ?? { last: run, stopped: false, spent: 0 }; + const attempt = attempts.get(key) ?? { last: run, stopped: false, killed: false, spent: 0 }; if (startOf(run) >= startOf(attempt.last)) { attempt.last = run; } attempt.spent += 1; - attempt.stopped ||= stoppedAt(run); + attempt.stopped ||= stoppedAt(run, stops); + attempt.killed ||= stoppedAt(run, kills); attempts.set(key, attempt); } - return [...attempts.values()].map(({ last, stopped, spent }) => ({ ...fromRest(last), ...(stopped ? { conclusion: "FAILURE" } : {}), spent })); + const as = (stopped, killed) => (stopped ? { conclusion: "FAILURE" } : killed ? { conclusion: "NEUTRAL", title: KILL_SWITCH_STOP } : {}); + return [...attempts.values()].map(({ last, stopped, killed, spent }) => ({ ...fromRest(last), ...as(stopped, killed), spent })); } // The fix-ci runs that count: an untrusted-trigger skip ran nothing. const counted = (runs) => runs.filter((run) => !(run.conclusion === "NEUTRAL" && String(run.title ?? "").startsWith("Skipped: untrusted-trigger"))); -// A run that failed, found no failing check to fix, or skipped on a PR held -// then may be retried; any other run (going, pushed a fix, escalated) handles -// the head. -const heldSkip = (run) => run.conclusion === "NEUTRAL" && run.title === HELD_SKIP; -const retryable = (run) => run.status === "COMPLETED" && (run.conclusion === "FAILURE" || heldSkip(run) || (run.conclusion === "NEUTRAL" && run.title === NOT_APPLICABLE)); +// A run that failed, found no failing check to fix, skipped on a PR held +// then, or that the kill switch stopped (paused) may be retried; any other +// run (going, pushed a fix, escalated) handles the head. +const paused = (run) => run.conclusion === "NEUTRAL" && (run.title === HELD_SKIP || run.title === KILL_SWITCH_STOP); +const retryable = (run) => run.status === "COMPLETED" && (run.conclusion === "FAILURE" || paused(run) || (run.conclusion === "NEUTRAL" && run.title === NOT_APPLICABLE)); // The number of counted fix-ci model runs when every attempt may be retried // and they reach maxRunsPerHead, the head's hard end; else 0. An attempt @@ -418,8 +423,9 @@ export function skipReason( if (runs.length === 0) { return ""; } - // A held skip (a held PR stopped above) counts toward the per-head cap with - // the rest, so a hold the run reads and this dispatcher does not stops there. + // A held skip (a held PR stopped above) or a kill-switch stop counts toward + // the per-head cap with the rest, so a hold the run reads and this + // dispatcher does not, or a switch that keeps flipping mid-run, stops there. if (!runs.every(retryable)) { return `head already has a ${checkName} check run`; } @@ -431,7 +437,7 @@ export function skipReason( } // Only a run on the current base tip counts: one that ended before the tip // was committed ran against an older base (an unreadable time counts). - const current = runs.filter((run) => !heldSkip(run) && !(Date.parse(run.completedAt ?? "") < baseAt)); + const current = runs.filter((run) => !paused(run) && !(Date.parse(run.completedAt ?? "") < baseAt)); if (current.some((run) => run.conclusion !== "FAILURE")) { return `head's ${checkName} found no failing check on this base`; } diff --git a/.github/scripts/pr-unblock/fix-ci-dispatch.test.mjs b/.github/scripts/pr-unblock/fix-ci-dispatch.test.mjs index 8ce3799594..90aea50dcd 100644 --- a/.github/scripts/pr-unblock/fix-ci-dispatch.test.mjs +++ b/.github/scripts/pr-unblock/fix-ci-dispatch.test.mjs @@ -1266,6 +1266,25 @@ test("a fix-ci run its push job stopped is retried on the same head once, then l assert.ok(capped.lines.some((line) => line.includes("head already has a pr-unblock / fix-ci check run")), capped.lines.join("\n")); }); +// A kill switch turned on after the fix-ci model run stops its push job at +// the broker (request-lane-token kill-switch-ok): the apply-patch check skips +// kill-switch, so the attempt did no work. It is retried like a held skip, +// spending no fix-ci restart, and so is a kill-switch stop of fix-ci itself. +test("a fix-ci run the kill switch stopped is retried on the same head without spending a restart", async () => { + const head = "7".padStart(40, "a"); + const at = (time) => `2026-10-22T${time}:00Z`; + const fixCi = (time) => ({ name: "pr-unblock / fix-ci", status: "completed", conclusion: "success", output: { title: "Fixed" }, started_at: at(time), completed_at: at(time) }); + const killed = (name, time) => ({ name, status: "completed", conclusion: "neutral", output: { title: "Skipped: kill-switch" }, started_at: at(time), completed_at: at(time) }); + const twice = await run({ + pages: [[pr(7)]], + history: new Map([[head, [fixCi("10:00"), fixCi("10:30")]]]), + stops: new Map([[head, [killed("pr-unblock / apply-patch", "10:05"), killed("pr-unblock / apply-patch", "10:35")]]]), + }); + assert.ok(twice.lines.some((line) => line.startsWith("dispatched fix-ci #7 ")), twice.lines.join("\n")); + const own = await run({ pages: [[pr(7)]], history: new Map([[head, [killed("pr-unblock / fix-ci", "10:00")]]]) }); + assert.ok(own.lines.some((line) => line.startsWith("dispatched fix-ci #7 ")), own.lines.join("\n")); +}); + // fix-ci and the in-run fix-ci-rung-up each post a fix-ci check in one // workflow run (the run id in each check's external_id; the API returns each // check's details_url as the check run's own page). When the rung-up's diff --git a/.github/scripts/pr-unblock/limit-stop.mjs b/.github/scripts/pr-unblock/limit-stop.mjs index 0eed6a21f7..80047fec14 100644 --- a/.github/scripts/pr-unblock/limit-stop.mjs +++ b/.github/scripts/pr-unblock/limit-stop.mjs @@ -94,10 +94,25 @@ const heldSkip = (run) => run.status === "completed" && run.conclusion === "neut // `failuresRetry` a failed run of any kind does too. Either way the head is // handled once `limit-stops-per-head` of them, limit stops included, stand on // it, so a dispatcher and a run that disagree about a hold stop there. -const released = (run, heldSkipsRelease) => heldSkipsRelease && heldSkip(run); - -// Whether `runs` handle the head: any run that is not an untrusted skip or a -// reset limit stop does, and so do `limit-stops-per-head` limit stops. +const released = (run, heldSkipsRelease) => (heldSkipsRelease && heldSkip(run)) || killSwitchStop(run); + +// The neutral check a run the lanes kill switch stopped posts: its own +// kill-switch step, or the broker refusing its mint for a switch turned on +// after the run read its variables. The dispatchers run only with the switch +// off, so such a run never handles its head, and every dispatcher reads it +// like a held skip: it counts toward `limit-stops-per-head`, so a switch that +// keeps flipping mid-run still ends the head there. +export const KILL_SWITCH_STOP = "Skipped: kill-switch"; +export const killSwitchStop = (run) => run?.status === "completed" && run.conclusion === "neutral" && run.output?.title === KILL_SWITCH_STOP; + +// Whether an emitter (push, reply or verdict) check releases the head's +// earlier activity checks for a same-head retry: a lane stop, or a +// kill-switch stop, which did no work. +export const releasesHead = (run) => laneStop(run) || killSwitchStop(run); + +// Whether `runs` handle the head: any run that is not an untrusted skip, a +// kill-switch stop or a reset limit stop does, and so do +// `limit-stops-per-head` such stops. export function anyHandles(runs, { now, limits = LIMIT_DEFAULTS, failuresRetry = false, heldSkipsRelease = false }) { let stops = 0; for (const run of runs) { @@ -155,13 +170,13 @@ const startOf = (run) => Date.parse(run?.started_at ?? ""); // How a head's emitter checks (`stopRuns`, REST shape) release its activity // checks for a same-head retry: `cut` is the newest emitter check's start -// when that check is a lane stop and fewer than `cap` lane stops stand on the -// head (the dispatcher's own per-head cap), so activity checks that started -// before it stop counting; null otherwise. `stops` counts the lane stops. +// when that check releases the head (releasesHead) and fewer than `cap` such +// checks stand on it (the dispatcher's own per-head cap), so activity checks +// that started before it stop counting; null otherwise. `stops` counts them. export function stopRelease(stopRuns, cap) { - const stops = stopRuns.filter(laneStop).length; + const stops = stopRuns.filter(releasesHead).length; const newest = stopRuns.reduce((a, b) => (startOf(b) > startOf(a) ? b : a), stopRuns[0]); - return { cut: laneStop(newest) && stops < cap ? startOf(newest) : null, stops }; + return { cut: releasesHead(newest) && stops < cap ? startOf(newest) : null, stops }; } // `runs` without those started before `cut`, a limit stop kept. diff --git a/.github/scripts/pr-unblock/limit-stop.test.mjs b/.github/scripts/pr-unblock/limit-stop.test.mjs index bb03071021..cca1d2d5d1 100644 --- a/.github/scripts/pr-unblock/limit-stop.test.mjs +++ b/.github/scripts/pr-unblock/limit-stop.test.mjs @@ -1,7 +1,7 @@ import assert from "node:assert/strict"; import { test } from "node:test"; -import { anyHandles, limitResetsAt, pendingStop, permanentlyHandles } from "./limit-stop.mjs"; +import { anyHandles, KILL_SWITCH_STOP, killSwitchStop, limitResetsAt, pendingStop, permanentlyHandles, releasedBy, releasesHead, stopRelease } from "./limit-stop.mjs"; const HOUR = 3_600_000; const ENDED = "2026-10-17T10:00:00Z"; @@ -173,3 +173,44 @@ test("with held skips released, an awaiting-human skip does not handle the head" assert.equal(permanentlyHandles(three, { heldSkipsRelease: true }), true); assert.equal(anyHandles([heldSkip, run(1), run(2)], { now, heldSkipsRelease: true, failuresRetry: true }), true, "held skips and failures share the cap"); }); + +// A run the lanes kill switch stopped did no work: its own kill-switch step, +// or a broker refusal of its mint (request-lane-token kill-switch-ok), posts +// neutral kill-switch (decide.mjs: "Skipped: "). +const killed = (id = 20, started = "2026-10-17T09:00:00Z") => ({ + id, + status: "completed", + conclusion: "neutral", + started_at: started, + completed_at: ENDED, + output: { title: KILL_SWITCH_STOP }, +}); + +test("a kill-switch stop never handles a head, with or without the opt-ins, until three stops stand on it", () => { + const now = ended + HOUR; + assert.equal(KILL_SWITCH_STOP, "Skipped: kill-switch"); + assert.equal(killSwitchStop(killed()), true); + assert.equal(killSwitchStop({ ...killed(), conclusion: "failure" }), false, "a red kill-switch denial is no such stop"); + assert.equal(killSwitchStop({ ...killed(), output: { title: "Skipped: prerequisite-missing" } }), false); + for (const options of [{}, { heldSkipsRelease: true, failuresRetry: true }]) { + assert.equal(anyHandles([killed()], { now, ...options }), false); + assert.equal(permanentlyHandles([killed()], options), false); + const three = [killed(20), killed(21), killed(22)]; + assert.equal(anyHandles(three, { now, ...options }), true, "a switch that keeps flipping mid-run ends the head at the cap"); + } + assert.equal(anyHandles([killed(), run(1), run(2)], { now, failuresRetry: true }), true, "kill-switch stops and failures share the cap"); + assert.equal(anyHandles([killed(), { ...killed(30), conclusion: "success", output: { title: "Passed" } }], { now }), true, "another run still handles the head"); +}); + +test("an emitter check's kill-switch stop releases the head's earlier activity checks, like a lane stop", () => { + const activity = { ...killed(1, "2026-10-17T08:00:00Z"), conclusion: "success", output: { title: "Passed" } }; + const stop = killed(2, "2026-10-17T08:30:00Z"); + assert.equal(releasesHead(stop), true); + const { cut, stops } = stopRelease([stop], 3); + assert.equal(cut, Date.parse("2026-10-17T08:30:00Z")); + assert.equal(stops, 1); + assert.deepEqual(releasedBy([activity], cut), []); + const capped = stopRelease([killed(2), killed(3), killed(4)], 3); + assert.equal(capped.cut, null, "the dispatcher's per-head cap ends the retries"); + assert.equal(stopRelease([{ ...stop, output: { title: "Passed" } }], 3).cut, null, "a push that acted releases nothing"); +}); diff --git a/.github/workflows/pr-run-activity-write.yml b/.github/workflows/pr-run-activity-write.yml index 190507c952..7c11acc4d0 100644 --- a/.github/workflows/pr-run-activity-write.yml +++ b/.github/workflows/pr-run-activity-write.yml @@ -1044,12 +1044,20 @@ jobs: # of a workflow file that GitHub's compare of the default branch with # the gated head lists as changed by this PR ends it green with no token # and skip reason workflow-modified, so every later step skips and the - # check is neutral. Every later step keys on the token. + # check is neutral. Every later step keys on the token. A PR run the + # broker refuses `kill-switch` (a switch turned on after the kill step + # above read it) ends the same way, with skip reason kill-switch, so the + # check is neutral and the dispatchers re-fire the head once the switch + # lifts (limit-stop.mjs killSwitchStop). An issue run stays red: + # work-unblock re-fires its issue from the failed mint step. So does + # report-escalation: no dispatcher re-fires it, so a skip would drop + # the escalation's comment and label. - name: Request the lane token id: lane-token if: steps.head-recheck.outputs.proceed == 'true' uses: ./.base/.github/actions/request-lane-token # zizmor: ignore[self-repository] with: + kill-switch-ok: ${{ inputs.issue-number == '' && inputs.activity != 'report-escalation' }} pr-workflow-change-ok: ${{ inputs.issue-number == '' && github.event_name == 'pull_request' }} head-sha: ${{ steps.gate.outputs.head-sha }} default-branch: ${{ github.event.repository.default_branch }} @@ -1582,7 +1590,7 @@ jobs: # It is also uploaded below as audit evidence, since the REST API cannot # read step summaries; nothing in this workflow reads that artifact. - name: Show the skill's final reply - if: always() && (steps.act-skill.outcome == 'success' || steps.act-skill.outcome == 'failure') + if: always() && steps.lane-token.outputs.token != '' && (steps.act-skill.outcome == 'success' || steps.act-skill.outcome == 'failure') env: EXECUTION_FILE: >- ${{ steps.act-skill.outputs.execution_file @@ -1615,7 +1623,7 @@ jobs: # step runs in pr-run-activity-read.yml, which holds no lane token. - name: Check the skill took a model turn id: act-turns - if: always() && (steps.act-skill.outcome == 'success' || steps.act-skill.outcome == 'failure') + if: always() && steps.lane-token.outputs.token != '' && (steps.act-skill.outcome == 'success' || steps.act-skill.outcome == 'failure') uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 env: EXECUTION_FILE: >- @@ -1789,7 +1797,7 @@ jobs: # changed PATH. The lane branch is deleted either way. - name: Move the PR branch to the lane's commits id: promote - if: always() && steps.lane-branch.outputs.branch != '' && inputs.issue-number == '' + if: always() && steps.lane-token.outputs.token != '' && steps.lane-branch.outputs.branch != '' && inputs.issue-number == '' uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 env: HEAD_SHA: ${{ steps.gate.outputs.head-sha }} diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index dcac699309..99deb1652e 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -786,7 +786,10 @@ missed), `prerequisite-missing` (among others, mergeability still unknown at the bounded re-reads), `cost-gated`, `awaiting-human` (among others, a `labels-absent` label is on the PR), `superseded-sha`, `disabled-by-config`, `untrusted-trigger` (a fork, no same-repository PR, or an actor or author not on the trusted-actor list), `workflow-modified` (the token broker refused a -write lane's mint for a workflow file the PR itself changes), `rung-up` (a fix-ci run's patch is +write lane's mint for a workflow file the PR itself changes), `kill-switch` (a lanes kill switch +stopped the run: its own kill-switch step, or the token broker refusing a write lane's mint on a +pull request for a switch turned on after that step; the pr-unblock dispatchers dispatch the head +again once the switch lifts), `rung-up` (a fix-ci run's patch is held back for one retry one model rung up, [Lanes](#lanes)), `no-model-turn` (an address-feedback round whose model never ran, so the scan uploaded no replies), `finding-stands` (an address-feedback round's reply job, when an adjudicator let a disputed finding stand and the head @@ -1505,7 +1508,10 @@ was turned on stops in the merge queue; owner and Dependabot pull requests are n Once the switch is lifted, the next `maintenance-reconcile-lost-runs` run (half-hourly) re-runs the failed `ci-status` of each open lanes App pull request whose newest checks run the switch failed on its first attempt, at most three per run, merge-critical first; while the switch is on it reads -nothing. +nothing. A pr-unblock run the switch stopped mid-run posts its check neutral `kill-switch`, which no +dispatcher counts as handling the head, so the next dispatcher run after the lift (the half-hourly +`pr-unblock-dispatch.yml` tick, or a review completing at the head) dispatches it again, at most +`limits.limit-stops-per-head` times per head, `SWEEP_HEAD_CAP` for `address-feedback`. `ci-status` enforces the same holds on every lanes App pull request, the two text holds and the hold paths, so one added after arming stops the merge without waiting for this job: its step `Fail a lanes App pull request its body or a member holds` runs diff --git a/docs/conventions/pr-pipeline/pr-pipeline.schema.json b/docs/conventions/pr-pipeline/pr-pipeline.schema.json index 1d675c3bc0..cdd278ad2e 100644 --- a/docs/conventions/pr-pipeline/pr-pipeline.schema.json +++ b/docs/conventions/pr-pipeline/pr-pipeline.schema.json @@ -497,6 +497,7 @@ "research-first", "probe-first", "workflow-modified", + "kill-switch", "rung-up", "no-model-turn", "finding-stands", diff --git a/docs/conventions/pr-pipeline/pr-run-activity.md b/docs/conventions/pr-pipeline/pr-run-activity.md index 43acdd83f1..5ad27b9edb 100644 --- a/docs/conventions/pr-pipeline/pr-run-activity.md +++ b/docs/conventions/pr-pipeline/pr-run-activity.md @@ -171,8 +171,8 @@ times out after 10 minutes. In order: Any answer but an allow, or no answer, fails the job, which skips the run job. The report job passes the broker-check job's result as `run-result` whenever it is not `success`, so a failure posts a failure check and a cancel follows `report-check-run`'s cancel rule. A broker denial is -red, not neutral, for the same reason as the write file's `kill-switch` denial: step 3 already -stopped the common case. Off `pull_request`, a failed broker-check job posts no check, since no +red, not neutral, `kill-switch` included: step 3 already stopped the common case. Off +`pull_request`, a failed broker-check job posts no check, since no head SHA was gated ([Runs that post no check](#runs-that-post-no-check)). The job outputs `allowed` (`true` only when step 4 succeeded) and `attempt` @@ -1881,8 +1881,17 @@ reason; no response at all is `broker-unreachable`. Broker reasons: | 503 | `github-rate-limited` | GitHub rate-limited the broker before it reserved the mint; `Retry-After` carries GitHub's wait (1-3600 s) and the body adds `limit`, `resource` and `retry_after`; retryable after that wait | | 503 | `mint-unavailable` | The mint failed after the broker reserved the job's one mint; final, never retried | -A `kill-switch` denial is red, not neutral: step 3 already stopped the common case, so the denial -means the job's read and the broker's read of the switch disagreed. +A `kill-switch` denial means the switch was turned on after the run job's own kill-switch step +read it (the broker reads the switches live). On a pull request run the write file passes +`kill-switch-ok`, so the denial is a designed stop: the token step ends green with no token, every +later step skips, and the check is neutral `kill-switch`. A caller job's dependents read the skip +from its `skip-reason` output. The pr-unblock dispatchers count a `kill-switch` check as a stop that +did no work (`limit-stop.mjs` `killSwitchStop`): on the activity's own check it does not handle the +head, and on a push, reply or verdict job's check it releases the head's earlier activity checks as +a lane stop does, both under the per-head caps. They run only with the switch off, so the head is +dispatched again once the switch lifts. An issue run's denial stays red, since work-unblock re-fires +its issue from the failed mint step, and so does a `report-escalation` run's, since no dispatcher +re-fires it and a skip would drop the escalation's comment and label. ### A workflow file the PR changes