diff --git a/.github/workflows/mcp-release-please.yml b/.github/workflows/mcp-release-please.yml index 8e903db9f..6a8577a03 100644 --- a/.github/workflows/mcp-release-please.yml +++ b/.github/workflows/mcp-release-please.yml @@ -343,36 +343,11 @@ jobs: # number of leading non-successes; no success in the window at all means the whole window is bad. # Escalates ONCE per outage by reusing an open issue instead of filing a new one per commit -- an # alert that repeats per attempt is the same unread noise in a different place. + # #10146: this was 30 lines of inline bash duplicating what selfhost.yml now also needs. Same class, + # same threshold, one implementation -- scripts/escalate-workflow-outage.ts. Behaviour is unchanged + # except the tracking issue's title, which is now workflow-generic rather than publish-specific. escalate_persistent_publish_failure() { - local workflow="$1" threshold=3 leading - # $c below is a JQ variable bound by `as $c`, so the single quotes are required -- letting the - # shell expand it would hand jq an empty filter. - # shellcheck disable=SC2016 - leading=$(gh api "repos/${GITHUB_REPOSITORY}/actions/workflows/${workflow}/runs?per_page=10&status=completed" \ - --jq '[.workflow_runs[].conclusion] as $c | (($c | index("success")) // ($c | length))' 2>/dev/null || echo 0) - if [ "${leading:-0}" -lt "$threshold" ]; then - echo "$workflow: ${leading:-0} consecutive failure(s) -- below the $threshold-commit escalation threshold, treating as transient." - return 0 - fi - local body - body=$(printf '%s\n' \ - "\`$workflow\` has failed on **${leading} consecutive runs**." \ - "" \ - "That is no longer a flake being retried -- a deterministic failure fails identically every time, so the package has not been publishing at all for that entire stretch. Retrying it further buys nothing." \ - "" \ - "Check the most recent run's logs, fix the cause, and close this issue; it is re-filed automatically only if the failure streak reaches ${threshold} again after a success." \ - "" \ - "Filed automatically by the release workflow (#9951).") - local existing - existing=$(gh issue list --state open --search "publish outage ${workflow} in:title" --json number --jq '.[0].number // empty' 2>/dev/null || echo "") - if [ -n "$existing" ]; then - echo "$workflow: standing outage already tracked in #$existing -- not filing a duplicate." - else - gh issue create --title "publish outage: $workflow has failed on consecutive commits" \ - --label maintainer-only --body "$body" >/dev/null \ - && echo "::error::$workflow has failed $leading consecutive runs -- standing outage filed." \ - || echo "::warning::$workflow standing outage detected but the tracking issue could not be filed." - fi + node --experimental-strip-types "${GITHUB_WORKSPACE}/scripts/escalate-workflow-outage.ts" --workflow "$1" } # packages/loopover-mcp and packages/loopover-miner carry REAL runtime `dependencies` entries on diff --git a/.github/workflows/selfhost.yml b/.github/workflows/selfhost.yml index 11fd188b7..0299c45b5 100644 --- a/.github/workflows/selfhost.yml +++ b/.github/workflows/selfhost.yml @@ -172,3 +172,33 @@ jobs: curl -sf http://127.0.0.1:8787/metrics | grep -q 'loopover_uptime_seconds' docker logs gt 2>&1 | grep -q 'selfhost_migrations_applied' echo "self-host smoke test passed" + + # #10146: a post-merge workflow going red blocks nothing and pages no one. This one caught migration 0209's + # SQLite-only AUTOINCREMENT correctly on the very first push (#10138) and then stayed red across five + # consecutive runs while PRs kept merging, because the only signal was a red check on a branch nobody was + # watching. #9951 had already solved this exact class for the publish workflows; the escalation is now a + # shared script rather than a second copy of it. + # + # Deliberately `failure()` and not `always()`: a success must never file anything. Deliberately consecutive + # -- one red run is a flake and alerting on it is how an alert gets muted. + escalate-persistent-failure: + name: escalate persistent failure + needs: build-boot + if: ${{ failure() }} + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: read + # The one thing this job does that build-boot cannot: file the tracking issue. + issues: write + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + persist-credentials: false + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7 + with: + node-version-file: .nvmrc + - name: Escalate if this workflow has failed on consecutive runs + env: + GH_TOKEN: ${{ github.token }} + run: node --experimental-strip-types scripts/escalate-workflow-outage.ts --workflow selfhost.yml diff --git a/scripts/escalate-workflow-outage.ts b/scripts/escalate-workflow-outage.ts new file mode 100644 index 000000000..af721645a --- /dev/null +++ b/scripts/escalate-workflow-outage.ts @@ -0,0 +1,116 @@ +#!/usr/bin/env node +// Escalate a workflow that has failed on CONSECUTIVE runs, once per outage (#10146, generalising #9951). +// +// #9951 built this for the publish workflows after they failed on every single main commit for as far back +// as the run history went -- a one-line missing build step -- while nobody noticed, because the only signal +// was a `::warning::` nobody reads and a red check that looks like release noise. +// +// The identical thing then happened to selfhost.yml. Migration 0209 shipped SQLite-only `AUTOINCREMENT` +// (#10138); the real-Postgres suite caught it correctly on the very first push, and the workflow stayed red +// across five consecutive runs while PRs kept merging, because a post-merge failure blocks nothing and pages +// no one. Two instances of one class is the point at which the mechanism belongs in one place instead of +// being reimplemented per workflow -- which is why this is a script both callers invoke rather than a second +// copy of the bash. +// +// ── A FLAKE AND AN OUTAGE ARE DIFFERENT THINGS ──────────────────────────────────────────────────────────── +// A deterministic failure fails identically every time, so a retry buys nothing and a single red run is not +// evidence of one. Consecutive failures at the HEAD of the run history are. Below the threshold this stays +// silent on purpose: an alert that fires on every transient red is the same unread noise in a new place. +// +// ── ONCE PER OUTAGE ─────────────────────────────────────────────────────────────────────────────────────── +// An open tracking issue is reused rather than a fresh one filed per commit, for the same reason. + +import { execFileSync } from "node:child_process"; + +/** + * PURE. How many runs at the head of the history did NOT succeed. + * + * `runs` is newest-first, as the GitHub API returns it. A window with no success anywhere means the whole + * window is bad -- that is the standing-outage case, and reporting `length` rather than 0 is what makes it + * escalate instead of silently reading as healthy. That distinction is the entire point: the naive + * `indexOf("success")` returns -1 there, and -1 treated as a count would report "no failures" for the worst + * possible state. + */ +export function leadingNonSuccessCount(runs: readonly (string | null | undefined)[]): number { + const firstSuccess = runs.findIndex((conclusion) => conclusion === "success"); + return firstSuccess === -1 ? runs.length : firstSuccess; +} + +/** The tracking issue's title for a workflow. Stable, and derived from the workflow file name, so the + * reuse-an-open-issue lookup below can find the one this outage already filed. */ +export function outageIssueTitle(workflow: string): string { + return `workflow outage: ${workflow} has failed on consecutive runs`; +} + +function gh(args: readonly string[]): string { + return execFileSync("gh", args, { encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] }).trim(); +} + +function outageBody(workflow: string, streak: number, threshold: number): string { + return [ + `\`${workflow}\` has failed on **${streak} consecutive runs**.`, + "", + "That is no longer a flake being retried -- a deterministic failure fails identically every time, so this", + "has been broken for that entire stretch and every run since the first one was already telling us so.", + "", + "Check the most recent run's logs, fix the cause, and close this issue. It is re-filed automatically only", + `if the failure streak reaches ${threshold} again after a success.`, + "", + "Filed automatically by scripts/escalate-workflow-outage.ts (#10146).", + ].join("\n"); +} + +function parseArg(name: string, fallback?: string): string { + const index = process.argv.indexOf(`--${name}`); + const value = index === -1 ? undefined : process.argv[index + 1]; + if (value === undefined || value.startsWith("--")) { + if (fallback !== undefined) return fallback; + console.error(`escalate-workflow-outage: --${name} is required`); + process.exit(2); + } + return value; +} + +function main(): void { + const workflow = parseArg("workflow"); + const threshold = Number(parseArg("threshold", "3")); + const repo = process.env.GITHUB_REPOSITORY ?? ""; + if (!repo) { + console.error("escalate-workflow-outage: GITHUB_REPOSITORY is not set"); + process.exit(2); + } + + let conclusions: (string | null)[] = []; + try { + conclusions = JSON.parse( + gh(["api", `repos/${repo}/actions/workflows/${workflow}/runs?per_page=10&status=completed`, "--jq", "[.workflow_runs[].conclusion]"]), + ) as (string | null)[]; + } catch (error) { + // Never fail the caller over the ALERTING path -- the workflow this runs in has already failed, and + // turning "could not check the streak" into a second red is pure noise on top of the real problem. + console.warn(`::warning::escalate-workflow-outage: could not read run history for ${workflow}: ${String(error)}`); + return; + } + + const streak = leadingNonSuccessCount(conclusions); + if (streak < threshold) { + console.log(`${workflow}: ${streak} consecutive failure(s) -- below the ${threshold}-run escalation threshold, treating as transient.`); + return; + } + + const title = outageIssueTitle(workflow); + try { + const existing = gh(["issue", "list", "--repo", repo, "--state", "open", "--search", `${title} in:title`, "--json", "number", "--jq", ".[0].number // empty"]); + if (existing) { + console.log(`${workflow}: standing outage already tracked in #${existing} -- not filing a duplicate.`); + return; + } + gh(["issue", "create", "--repo", repo, "--title", title, "--label", "maintainer-only", "--body", outageBody(workflow, streak, threshold)]); + console.log(`::error::${workflow} has failed ${streak} consecutive runs -- standing outage filed.`); + } catch (error) { + console.warn(`::warning::${workflow} standing outage detected but the tracking issue could not be filed: ${String(error)}`); + } +} + +// Only run when invoked directly, so the pure helpers above stay importable by tests. +if (process.argv[1] && import.meta.url === `file://${process.argv[1]}`) main(); diff --git a/test/unit/escalate-workflow-outage.test.ts b/test/unit/escalate-workflow-outage.test.ts new file mode 100644 index 000000000..88eec5d11 --- /dev/null +++ b/test/unit/escalate-workflow-outage.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, it } from "vitest"; + +import { leadingNonSuccessCount, outageIssueTitle } from "../../scripts/escalate-workflow-outage"; + +// #10146: a post-merge workflow going red blocks nothing and pages no one. +// +// #9951 built this counting for the publish workflows after they failed on every main commit for as far back +// as the run history went, unnoticed. The same thing then happened to selfhost.yml: it caught migration +// 0209's SQLite-only AUTOINCREMENT (#10138) correctly on the FIRST push and stayed red for five consecutive +// runs while PRs kept merging. Two instances of one class is when the mechanism belongs in one place, so the +// bash moved into a script both workflows call -- and the arithmetic that decides "flake or outage" is the +// part worth pinning, because getting it wrong in either direction destroys the alert's usefulness. + +describe("leadingNonSuccessCount (#10146)", () => { + it("counts the unbroken run of non-successes at the HEAD of the history", () => { + // Newest-first, as the GitHub API returns it. + expect(leadingNonSuccessCount(["failure", "failure", "success", "failure"])).toBe(2); + }); + + it("is zero when the most recent run succeeded, however bad the history behind it", () => { + // A fixed workflow must stop alerting immediately -- an alert that persists after the fix gets muted, + // and then the NEXT real outage is invisible. + expect(leadingNonSuccessCount(["success", "failure", "failure", "failure", "failure"])).toBe(0); + }); + + it("REGRESSION: a window with NO success anywhere reports the whole window, not zero", () => { + // The case the whole mechanism exists for, and the easiest to get backwards. `indexOf("success")` + // returns -1 here; treating that as a count reports "no failures" for the single worst possible state -- + // a workflow that has never once succeeded in its recorded history. That is precisely the shape #9951 + // found (publish red on every commit as far back as the history went) and the shape selfhost.yml was in + // for five runs. + expect(leadingNonSuccessCount(["failure", "failure", "failure"])).toBe(3); + expect(leadingNonSuccessCount(Array(10).fill("failure"))).toBe(10); + }); + + it("treats cancelled, timed_out and null as non-successes — only an actual success breaks the streak", () => { + // A cancelled or still-unrecorded run is not evidence the workflow works. Counting it as a success would + // silently reset the streak and suppress the alert. + expect(leadingNonSuccessCount(["cancelled", "timed_out", null, undefined, "failure", "success"])).toBe(5); + }); + + it("is zero for an empty history, so a brand-new workflow never alerts", () => { + expect(leadingNonSuccessCount([])).toBe(0); + }); + + it("does not treat a non-'success' string as success on a prefix match", () => { + expect(leadingNonSuccessCount(["successful", "success"])).toBe(1); + }); +}); + +describe("outageIssueTitle", () => { + it("is derived from the workflow file, so the reuse lookup finds the issue this outage already filed", () => { + // Filing once per outage instead of once per commit is the difference between an alert and noise, and it + // depends entirely on this string being stable and identifying. + expect(outageIssueTitle("selfhost.yml")).toBe("workflow outage: selfhost.yml has failed on consecutive runs"); + expect(outageIssueTitle("publish-mcp.yml")).not.toBe(outageIssueTitle("publish-miner.yml")); + }); +});