From b9ebd7097ab65968fb92f64a4a0f2e2abbddf689 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Fri, 31 Jul 2026 08:24:06 -0700 Subject: [PATCH] fix(ci): count only automation-dispatched runs toward a workflow outage `escalate-workflow-outage.ts` counted every non-success at the head of a workflow's run history, so a maintainer retrying a publish by hand escalated as a standing outage. That is what #10171 was: six consecutive publish-miner.yml failures, every one a hand `gh workflow run` against main failing ETARGET on a @loopover/contract version that was not published yet. The next run after contract landed succeeded with no code change. Filtering on the run's `event` / `triggering_actor` / `head_branch` cannot work here, and the header records why so it is not tried again. The reconcile path in mcp-release-please.yml dispatches a bare `gh workflow run "$workflow"` under a PAT, so its runs land as `workflow_dispatch` / `main` / the PAT owner -- the identical triple a laptop produces. Verified against live history: publish-miner run #484, which the reconcile job's own log shows it dispatched, is indistinguishable on every one of those fields from the six manual #10171 failures, and `GET /actions/runs/:id` exposes no `inputs` key either. So provenance is stamped at dispatch instead. `run-name:` is rendered into `display_title`, which the runs API does return -- the same mechanism visual-capture-fallback.yml already uses to correlate a dispatch to its PR. The five publish workflows take a `dispatched_by_automation` input and stamp a marker into their run name; both dispatch sites pass it; the streak counts only runs carrying it. Manual runs are excluded rather than merely not resetting the streak: a run nobody automated is not evidence about the automated path in either direction. Non-dispatch triggers still count exactly as before, so selfhost.yml -- the other caller, and push-triggered -- is unchanged. Runs already in the history carry no stamp and so read as unattributable. The publish escalation therefore stays quiet until three stamped automated failures accumulate; under-alerting briefly is the fail-safe direction and beats re-creating the false alarm this removes. check-dispatch-provenance-stamped.ts asserts the lockstep, since both sides of the marker fail silently: a drifted `run-name:` leaves the escalation reading every automated run as manual, still green and still "wired" while covering nothing. It also rejects an unconditional marker, which would stamp a human's dispatch as automated and restore the original false alarm. Closes #10234 --- .github/workflows/mcp-release-please.yml | 12 +- .github/workflows/publish-contract.yml | 12 ++ .github/workflows/publish-engine.yml | 12 ++ .github/workflows/publish-mcp.yml | 12 ++ .github/workflows/publish-miner.yml | 12 ++ .github/workflows/publish-ui-kit.yml | 12 ++ package.json | 3 +- scripts/check-dispatch-provenance-stamped.ts | 118 +++++++++++++ scripts/escalate-workflow-outage.ts | 102 +++++++++-- .../check-dispatch-provenance-stamped.test.ts | 161 ++++++++++++++++++ test/unit/escalate-workflow-outage.test.ts | 107 +++++++++++- 11 files changed, 533 insertions(+), 30 deletions(-) create mode 100644 scripts/check-dispatch-provenance-stamped.ts create mode 100644 test/unit/check-dispatch-provenance-stamped.test.ts diff --git a/.github/workflows/mcp-release-please.yml b/.github/workflows/mcp-release-please.yml index 6a8577a03a..b861039775 100644 --- a/.github/workflows/mcp-release-please.yml +++ b/.github/workflows/mcp-release-please.yml @@ -178,31 +178,31 @@ jobs: if: ${{ steps.release.outputs['packages/loopover-mcp--release_created'] == 'true' }} env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: gh workflow run publish-mcp.yml --ref "${{ steps.release.outputs['packages/loopover-mcp--tag_name'] }}" -f released_by_release_please=true + run: gh workflow run publish-mcp.yml --ref "${{ steps.release.outputs['packages/loopover-mcp--tag_name'] }}" -f released_by_release_please=true -f dispatched_by_automation=true - name: Dispatch Engine publish if: ${{ steps.release.outputs['packages/loopover-engine--release_created'] == 'true' }} env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: gh workflow run publish-engine.yml --ref "${{ steps.release.outputs['packages/loopover-engine--tag_name'] }}" -f released_by_release_please=true + run: gh workflow run publish-engine.yml --ref "${{ steps.release.outputs['packages/loopover-engine--tag_name'] }}" -f released_by_release_please=true -f dispatched_by_automation=true - name: Dispatch Contract publish if: ${{ steps.release.outputs['packages/loopover-contract--release_created'] == 'true' }} env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: gh workflow run publish-contract.yml --ref "${{ steps.release.outputs['packages/loopover-contract--tag_name'] }}" -f released_by_release_please=true + run: gh workflow run publish-contract.yml --ref "${{ steps.release.outputs['packages/loopover-contract--tag_name'] }}" -f released_by_release_please=true -f dispatched_by_automation=true - name: Dispatch Miner publish if: ${{ steps.release.outputs['packages/loopover-miner--release_created'] == 'true' }} env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: gh workflow run publish-miner.yml --ref "${{ steps.release.outputs['packages/loopover-miner--tag_name'] }}" -f released_by_release_please=true + run: gh workflow run publish-miner.yml --ref "${{ steps.release.outputs['packages/loopover-miner--tag_name'] }}" -f released_by_release_please=true -f dispatched_by_automation=true - name: Dispatch UI Kit publish if: ${{ steps.release.outputs['packages/loopover-ui-kit--release_created'] == 'true' }} env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: gh workflow run publish-ui-kit.yml --ref "${{ steps.release.outputs['packages/loopover-ui-kit--tag_name'] }}" -f released_by_release_please=true + run: gh workflow run publish-ui-kit.yml --ref "${{ steps.release.outputs['packages/loopover-ui-kit--tag_name'] }}" -f released_by_release_please=true -f dispatched_by_automation=true # Self-heal for a known, reproducible, upstream googleapis/release-please limitation # (googleapis/release-please#1946, #1444, #1406 -- all the same "There are untagged, merged release @@ -282,7 +282,7 @@ jobs: # `before_ts` only on the rare case the URL wasn't returned. local before_ts run_id dispatch_output before_ts="$(date -u +%Y-%m-%dT%H:%M:%SZ)" - dispatch_output="$(gh workflow run "$workflow" --repo "$GITHUB_REPOSITORY" 2>&1)" + dispatch_output="$(gh workflow run "$workflow" --repo "$GITHUB_REPOSITORY" -f dispatched_by_automation=true 2>&1)" echo "$dispatch_output" run_id="$(printf '%s' "$dispatch_output" | grep -oE '/runs/[0-9]+' | grep -oE '[0-9]+' | tail -1 || true)" if [ -z "$run_id" ]; then diff --git a/.github/workflows/publish-contract.yml b/.github/workflows/publish-contract.yml index fa0de5c8bf..f183348a9c 100644 --- a/.github/workflows/publish-contract.yml +++ b/.github/workflows/publish-contract.yml @@ -23,6 +23,18 @@ on: description: "Internal: set by the release automation's dispatch so this run skips re-creating the GitHub release it already made." type: boolean default: false + dispatched_by_automation: + description: "Internal: set by the release automation's dispatch so a hand retry is not counted as an outage. Informational -- it feeds run-name below and nothing else." + type: boolean + default: false + +# PROVENANCE STAMP (#10234). `run-name` is the ONLY dispatch-time signal the runs API gives back (as +# `display_title`): the reconcile path dispatches bare against main under a PAT, so `event`, `head_branch` +# and `triggering_actor` are identical to a human's `gh workflow run`, and run objects carry no `inputs`. +# scripts/escalate-workflow-outage.ts recovers "was this automated?" from the marker below -- see its +# header for the full story. The marker must stay in lockstep with that script's +# AUTOMATION_RUN_NAME_MARKER; scripts/check-dispatch-provenance-stamped.ts fails the build if they drift. +run-name: "Publish Contract Package${{ inputs.dispatched_by_automation && ' [automated]' || '' }}" permissions: contents: read diff --git a/.github/workflows/publish-engine.yml b/.github/workflows/publish-engine.yml index 49c6e00f53..0e784c7b0d 100644 --- a/.github/workflows/publish-engine.yml +++ b/.github/workflows/publish-engine.yml @@ -18,6 +18,18 @@ on: description: "Internal: set by the release automation's dispatch so this run skips re-creating the GitHub release it already made." type: boolean default: false + dispatched_by_automation: + description: "Internal: set by the release automation's dispatch so a hand retry is not counted as an outage. Informational -- it feeds run-name below and nothing else." + type: boolean + default: false + +# PROVENANCE STAMP (#10234). `run-name` is the ONLY dispatch-time signal the runs API gives back (as +# `display_title`): the reconcile path dispatches bare against main under a PAT, so `event`, `head_branch` +# and `triggering_actor` are identical to a human's `gh workflow run`, and run objects carry no `inputs`. +# scripts/escalate-workflow-outage.ts recovers "was this automated?" from the marker below -- see its +# header for the full story. The marker must stay in lockstep with that script's +# AUTOMATION_RUN_NAME_MARKER; scripts/check-dispatch-provenance-stamped.ts fails the build if they drift. +run-name: "Publish Engine Package${{ inputs.dispatched_by_automation && ' [automated]' || '' }}" permissions: contents: read diff --git a/.github/workflows/publish-mcp.yml b/.github/workflows/publish-mcp.yml index d5ee08ea3d..70bc49ccd1 100644 --- a/.github/workflows/publish-mcp.yml +++ b/.github/workflows/publish-mcp.yml @@ -12,6 +12,18 @@ on: description: "Internal: set by the release automation's dispatch so this run skips re-creating the GitHub release it already made." type: boolean default: false + dispatched_by_automation: + description: "Internal: set by the release automation's dispatch so a hand retry is not counted as an outage. Informational -- it feeds run-name below and nothing else." + type: boolean + default: false + +# PROVENANCE STAMP (#10234). `run-name` is the ONLY dispatch-time signal the runs API gives back (as +# `display_title`): the reconcile path dispatches bare against main under a PAT, so `event`, `head_branch` +# and `triggering_actor` are identical to a human's `gh workflow run`, and run objects carry no `inputs`. +# scripts/escalate-workflow-outage.ts recovers "was this automated?" from the marker below -- see its +# header for the full story. The marker must stay in lockstep with that script's +# AUTOMATION_RUN_NAME_MARKER; scripts/check-dispatch-provenance-stamped.ts fails the build if they drift. +run-name: "Publish MCP Package${{ inputs.dispatched_by_automation && ' [automated]' || '' }}" permissions: contents: read diff --git a/.github/workflows/publish-miner.yml b/.github/workflows/publish-miner.yml index c03fc95a06..32eff0ba27 100644 --- a/.github/workflows/publish-miner.yml +++ b/.github/workflows/publish-miner.yml @@ -18,6 +18,18 @@ on: description: "Internal: set by the release automation's dispatch so this run skips re-creating the GitHub release it already made." type: boolean default: false + dispatched_by_automation: + description: "Internal: set by the release automation's dispatch so a hand retry is not counted as an outage. Informational -- it feeds run-name below and nothing else." + type: boolean + default: false + +# PROVENANCE STAMP (#10234). `run-name` is the ONLY dispatch-time signal the runs API gives back (as +# `display_title`): the reconcile path dispatches bare against main under a PAT, so `event`, `head_branch` +# and `triggering_actor` are identical to a human's `gh workflow run`, and run objects carry no `inputs`. +# scripts/escalate-workflow-outage.ts recovers "was this automated?" from the marker below -- see its +# header for the full story. The marker must stay in lockstep with that script's +# AUTOMATION_RUN_NAME_MARKER; scripts/check-dispatch-provenance-stamped.ts fails the build if they drift. +run-name: "Publish Miner Package${{ inputs.dispatched_by_automation && ' [automated]' || '' }}" permissions: contents: read diff --git a/.github/workflows/publish-ui-kit.yml b/.github/workflows/publish-ui-kit.yml index 3d71b3cbcb..dba846ff2f 100644 --- a/.github/workflows/publish-ui-kit.yml +++ b/.github/workflows/publish-ui-kit.yml @@ -18,6 +18,18 @@ on: description: "Internal: set by the release automation's dispatch so this run skips re-creating the GitHub release it already made." type: boolean default: false + dispatched_by_automation: + description: "Internal: set by the release automation's dispatch so a hand retry is not counted as an outage. Informational -- it feeds run-name below and nothing else." + type: boolean + default: false + +# PROVENANCE STAMP (#10234). `run-name` is the ONLY dispatch-time signal the runs API gives back (as +# `display_title`): the reconcile path dispatches bare against main under a PAT, so `event`, `head_branch` +# and `triggering_actor` are identical to a human's `gh workflow run`, and run objects carry no `inputs`. +# scripts/escalate-workflow-outage.ts recovers "was this automated?" from the marker below -- see its +# header for the full story. The marker must stay in lockstep with that script's +# AUTOMATION_RUN_NAME_MARKER; scripts/check-dispatch-provenance-stamped.ts fails the build if they drift. +run-name: "Publish UI Kit Package${{ inputs.dispatched_by_automation && ' [automated]' || '' }}" permissions: contents: read diff --git a/package.json b/package.json index 96771d876a..4050a19383 100644 --- a/package.json +++ b/package.json @@ -101,6 +101,7 @@ "regate-sort-key:check": "node --experimental-strip-types scripts/check-regate-sort-key.ts", "command-redelivery-guards:check": "node --experimental-strip-types scripts/check-command-redelivery-guards.ts", "dispatch-gate-reasons:check": "node --experimental-strip-types scripts/check-dispatch-gate-reasons.ts", + "dispatch-provenance:check": "tsx scripts/check-dispatch-provenance-stamped.ts", "replay-runner-manifest": "tsx scripts/replay-runner-image-manifest.ts", "replay-runner-manifest:write": "tsx scripts/replay-runner-image-manifest.ts --write", "replay-runner-manifest:check": "tsx scripts/replay-runner-image-manifest.ts --check", @@ -149,7 +150,7 @@ "test:smoke:browser:install": "playwright install chromium", "test:smoke:browser": "node --experimental-strip-types scripts/smoke-ui-browser.ts", "pretest:ci": "npm run check-node-version", - "test:ci": "git diff --check && npm run actionlint && npm run lint:composite-actions && npm run db:migrations:check && npm run db:migrations:immutable:check && npm run turbo-inputs:check && npm run workspace-dep-ranges:check && npm run db:schema-drift:check && npm run selfhost:env-reference:check && npm run miner:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run build --workspace @loopover/engine && npm run build --workspace @loopover/discovery-index && npm run build:mcp && npm run build:miner && npm run build --workspace @loopover/ui-kit && npm run typecheck && npm run test:coverage && npm run test:engine-parity && npm run test:live-gate-parity && npm run test:driver-parity && npm run validate:mcp && npm run test --workspace @loopover/engine && npm run test:workers && npm run test:mcp-pack && npm run test:contract-pack && npm run test:miner-pack && npm run test:engine-pack && npm run test:ui-kit-pack && npm run test:miner-deployment-docs-audit && npm run rees:test && npm run ui:openapi:check && npm run control-plane:contract:check && npm run control-plane:openapi:check && npm run ui:version-audit && npm run docs:drift-check && npm run coverage-boltons:check && npm run import-specifiers:check && npm run ui-derived-types:check && npm run server-manifest:check && npm run dead-source-files:check && npm run dead-exports:check && npm run publishable-deps:check && npm run fixture-clock-races:check && npm run typecheck-coverage:check && npm run test-wiring:check && npm run checkers-wired:check && npm run focus-manifest-fields:check && npm run release-commit-parsing:check && npm run releasable-commit-types:check && npm run regate-sort-key:check && npm run maintainer-associations:check && npm run command-redelivery-guards:check && npm run dispatch-gate-reasons:check && npm run validate:no-hand-written-js && npm run replay-runner-manifest:check && npm run coco-dev-versions:check && npm run branding-drift:check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run engines-nvmrc:check && npm run release-manifest:sync:check && npm run release-linked-versions:check && npm run command-reference:check && npm run mcp:tool-reference:check && npm run mcp:client-config:check && npm run contract:api-schemas:check && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build", + "test:ci": "git diff --check && npm run actionlint && npm run lint:composite-actions && npm run db:migrations:check && npm run db:migrations:immutable:check && npm run turbo-inputs:check && npm run workspace-dep-ranges:check && npm run db:schema-drift:check && npm run selfhost:env-reference:check && npm run miner:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run build --workspace @loopover/engine && npm run build --workspace @loopover/discovery-index && npm run build:mcp && npm run build:miner && npm run build --workspace @loopover/ui-kit && npm run typecheck && npm run test:coverage && npm run test:engine-parity && npm run test:live-gate-parity && npm run test:driver-parity && npm run validate:mcp && npm run test --workspace @loopover/engine && npm run test:workers && npm run test:mcp-pack && npm run test:contract-pack && npm run test:miner-pack && npm run test:engine-pack && npm run test:ui-kit-pack && npm run test:miner-deployment-docs-audit && npm run rees:test && npm run ui:openapi:check && npm run control-plane:contract:check && npm run control-plane:openapi:check && npm run ui:version-audit && npm run docs:drift-check && npm run coverage-boltons:check && npm run import-specifiers:check && npm run ui-derived-types:check && npm run server-manifest:check && npm run dead-source-files:check && npm run dead-exports:check && npm run publishable-deps:check && npm run fixture-clock-races:check && npm run typecheck-coverage:check && npm run test-wiring:check && npm run checkers-wired:check && npm run focus-manifest-fields:check && npm run release-commit-parsing:check && npm run releasable-commit-types:check && npm run regate-sort-key:check && npm run maintainer-associations:check && npm run command-redelivery-guards:check && npm run dispatch-gate-reasons:check && npm run dispatch-provenance:check && npm run validate:no-hand-written-js && npm run replay-runner-manifest:check && npm run coco-dev-versions:check && npm run branding-drift:check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run engines-nvmrc:check && npm run release-manifest:sync:check && npm run release-linked-versions:check && npm run command-reference:check && npm run mcp:tool-reference:check && npm run mcp:client-config:check && npm run contract:api-schemas:check && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build", "test:release": "npm run test:ci && npm run changelog:check", "test:release:mcp": "npm run test:ci", "test:watch": "vitest", diff --git a/scripts/check-dispatch-provenance-stamped.ts b/scripts/check-dispatch-provenance-stamped.ts new file mode 100644 index 0000000000..1fdb56e55d --- /dev/null +++ b/scripts/check-dispatch-provenance-stamped.ts @@ -0,0 +1,118 @@ +// The dispatch-provenance stamp is a TWO-SIDED string, and both sides fail silently (#10234). +// +// scripts/escalate-workflow-outage.ts decides "was this run automated?" by looking for +// AUTOMATION_RUN_NAME_MARKER in the run's `display_title`. The publish workflows put it there via +// `run-name:`. Nothing at runtime notices when those two drift apart -- the escalation simply reads every +// automated run as manual, filters them all out, and goes permanently quiet. The alert would still be +// green, still be "wired", and would never fire again. +// +// That is the same shape as #9860's checkers-that-guard-nothing: a mechanism whose presence is read as +// coverage while it covers nothing. So the lockstep is asserted mechanically here rather than left to +// whoever next edits a `run-name:` remembering an unwritten obligation. +// +// Three invariants, each one a real way to break it: +// +// 1. A workflow declaring the provenance input MUST stamp the marker into `run-name:`. +// Breaks by: editing or deleting the run-name, or renaming the marker on the script side only. +// 2. That `run-name:` MUST be conditional on the input. An unconditional marker stamps EVERY run, +// including a human's, which restores the exact #10171 false alarm this all exists to remove. +// 3. Every dispatch site targeting such a workflow MUST pass the flag. Breaks by: adding a new +// `gh workflow run` and not knowing the flag exists -- those runs then read as manual. + +import { readdirSync, readFileSync } from "node:fs"; +import { join } from "node:path"; + +import { AUTOMATION_RUN_NAME_MARKER } from "./escalate-workflow-outage"; + +const WORKFLOWS_DIR = ".github/workflows"; + +/** The `workflow_dispatch` input carrying provenance. Must match what mcp-release-please.yml passes. */ +export const PROVENANCE_INPUT = "dispatched_by_automation"; + +/** True when this workflow declares the provenance input, i.e. it opts in to being stamped. */ +export function declaresProvenanceInput(text: string): boolean { + return new RegExp(`^\\s+${PROVENANCE_INPUT}:\\s*$`, "m").test(text); +} + +/** The top-level `run-name:` value, or undefined when the workflow has none. */ +export function runNameValue(text: string): string | undefined { + return /^run-name:[ \t]*(.+)$/m.exec(text)?.[1]?.trim(); +} + +/** + * PURE. Everything wrong with one workflow's stamp. Empty for a workflow that does not opt in, so this + * stays silent about push-triggered workflows (selfhost.yml) that have no dispatch ambiguity to resolve. + */ +export function stampProblems(text: string, marker: string): string[] { + if (!declaresProvenanceInput(text)) return []; + const runName = runNameValue(text); + if (runName === undefined) { + return [`declares the \`${PROVENANCE_INPUT}\` input but has no top-level \`run-name:\`, so nothing reaches \`display_title\` and every run reads as manual`]; + } + const problems: string[] = []; + if (!runName.includes(marker)) { + problems.push(`\`run-name:\` does not contain \`${marker}\`, the marker escalate-workflow-outage.ts searches \`display_title\` for`); + } + if (!runName.includes(`inputs.${PROVENANCE_INPUT}`)) { + problems.push(`\`run-name:\` does not branch on \`inputs.${PROVENANCE_INPUT}\`, so a HUMAN dispatch would be stamped as automated too`); + } + return problems; +} + +/** + * PURE. Dispatch sites in `text` that target a provenance-stamped workflow without passing the flag. + * + * A `$`-bearing target is a shell variable the reconcile path resolves at runtime (`gh workflow run + * "$workflow"`), so it cannot be matched against `provenanceWorkflows` statically and is required to pass + * the flag unconditionally. That is deliberate: the reconcile path is the ONE that calls the escalation, + * and an unstamped dispatch there is precisely the blind spot this file exists to prevent. + */ +export function dispatchesMissingFlag(text: string, provenanceWorkflows: ReadonlySet): string[] { + const missing: string[] = []; + for (const line of text.split("\n")) { + // Prose in these files discusses `gh workflow run` at length; a commented mention is not a dispatch. + if (line.trim().startsWith("#")) continue; + const match = /gh workflow run[ \t]+(\S+)(.*)$/.exec(line); + if (match === null) continue; + const target = match[1]!.replace(/^["']|["']$/g, ""); + const isRuntimeTarget = target.includes("$"); + if (!isRuntimeTarget && !provenanceWorkflows.has(target)) continue; + if (!match[2]!.includes(`-f ${PROVENANCE_INPUT}=true`)) missing.push(target); + } + return missing; +} + +function main(): void { + const files = readdirSync(WORKFLOWS_DIR).filter((entry) => entry.endsWith(".yml") || entry.endsWith(".yaml")); + const texts = new Map(files.map((file) => [file, readFileSync(join(WORKFLOWS_DIR, file), "utf8")])); + const stamped = new Set([...texts].filter(([, text]) => declaresProvenanceInput(text)).map(([file]) => file)); + + const failures: string[] = []; + for (const [file, text] of texts) { + for (const problem of stampProblems(text, AUTOMATION_RUN_NAME_MARKER)) { + failures.push(`${WORKFLOWS_DIR}/${file}: ${problem}`); + } + for (const target of dispatchesMissingFlag(text, stamped)) { + failures.push(`${WORKFLOWS_DIR}/${file}: dispatches \`${target}\` without \`-f ${PROVENANCE_INPUT}=true\`, so those runs read as a hand retry and are excluded from the outage streak`); + } + } + + if (failures.length > 0) { + console.error(`check-dispatch-provenance-stamped: ${failures.length} problem(s) — the outage escalation would silently stop seeing automated runs (#10234):\n`); + for (const failure of failures) console.error(` ${failure}`); + console.error( + [ + "", + `The marker is \`${AUTOMATION_RUN_NAME_MARKER}\`, exported as AUTOMATION_RUN_NAME_MARKER from scripts/escalate-workflow-outage.ts.`, + "Both sides must move together:", + ` • workflow — \`run-name: "\${{ inputs.${PROVENANCE_INPUT} && ' ${AUTOMATION_RUN_NAME_MARKER}' || '' }}"\``, + ` • dispatch — pass \`-f ${PROVENANCE_INPUT}=true\` from every automated \`gh workflow run\``, + ].join("\n"), + ); + process.exit(1); + } + + console.log(`check-dispatch-provenance-stamped: ${stamped.size} workflow(s) stamp dispatch provenance, and every automated dispatch passes the flag.`); +} + +if (process.argv[1]?.endsWith("check-dispatch-provenance-stamped.ts")) main(); diff --git a/scripts/escalate-workflow-outage.ts b/scripts/escalate-workflow-outage.ts index af721645ae..80edaff57c 100644 --- a/scripts/escalate-workflow-outage.ts +++ b/scripts/escalate-workflow-outage.ts @@ -19,21 +19,82 @@ // // ── ONCE PER OUTAGE ─────────────────────────────────────────────────────────────────────────────────────── // An open tracking issue is reused rather than a fresh one filed per commit, for the same reason. +// +// ── A HAND RETRY IS NOT AN OUTAGE (#10234) ──────────────────────────────────────────────────────────────── +// The premise above -- "a deterministic failure fails identically every time" -- holds just as well for a +// maintainer retrying a publish before its dependency is live, and only the first of those is an outage +// nobody is watching. #10171 was exactly that: six consecutive publish-miner.yml failures escalated to an +// issue, and every one was a hand `gh workflow run` against main, failing ETARGET on a @loopover/contract +// version that was not published yet. The next run after contract landed succeeded with no code change. +// The workflow was never broken. An alert that fires on the maintainer's own retries is the unread noise +// #9951 built this to escape, just relocated. +// +// ── WHY PROVENANCE HAS TO BE STAMPED ────────────────────────────────────────────────────────────────────── +// The obvious fix -- filter on the run's `event` / `triggering_actor` / `head_branch` -- does not work here, +// and it is worth recording why so it is not attempted again. The reconcile path in mcp-release-please.yml +// (`dispatch_and_wait`) issues a BARE `gh workflow run "$workflow"`: no `--ref`, no inputs, under a PAT. So +// its runs land as `workflow_dispatch` / `main` / the PAT owner -- the identical triple a laptop produces. +// Verified live: publish-miner run #484 (`30637452300`), which the reconcile job's own log shows it +// dispatched, is indistinguishable on every one of those fields from the six manual #10171 failures. And +// `GET /actions/runs/:id` carries no `inputs` key, so a dispatch input cannot be read back either. +// +// `run-name:` IS rendered into `display_title`, which the runs API does return -- the same trick +// visual-capture-fallback.yml already uses to correlate a dispatch back to its PR. So the publish workflows +// stamp AUTOMATION_RUN_NAME_MARKER into their run name when the automation dispatches them, and the streak +// below counts only runs carrying it. import { execFileSync } from "node:child_process"; /** - * PURE. How many runs at the head of the history did NOT succeed. + * The marker the publish workflows render into `run-name:` when the release automation dispatches them, + * and which `isAutomationDispatched` reads back out of `display_title`. + * + * Changing this string is a two-sided edit -- every `.github/workflows/publish-*.yml` `run-name:` must + * change with it, or every automated run silently reads as manual and the escalation goes permanently + * quiet. scripts/check-dispatch-provenance-stamped.ts fails the build if the two sides drift apart. + */ +export const AUTOMATION_RUN_NAME_MARKER = "[automated]"; + +/** The fields of a workflow run this script reads. Mirrors the GitHub runs API's own names. */ +export type WorkflowRunSummary = { + readonly conclusion: string | null | undefined; + readonly event: string | null | undefined; + readonly displayTitle: string | null | undefined; +}; + +/** + * PURE. Was this run started by automation rather than by a human at a terminal? * - * `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. + * Only `workflow_dispatch` is ambiguous. Every other trigger -- `push` (selfhost.yml), `schedule`, + * `workflow_run` -- is automation by construction, so it counts exactly as it did before #10234; that is + * what keeps this change from silently narrowing the selfhost.yml caller it also serves. + * + * The default for an unrecognised dispatch is therefore "manual", i.e. EXCLUDED. That is the deliberate + * direction: a maintainer's own failed dispatch is already visible to the maintainer who ran it, so + * dropping it costs nothing, while counting it re-creates #10171. + */ +export function isAutomationDispatched(run: WorkflowRunSummary): boolean { + if (run.event !== "workflow_dispatch") return true; + return (run.displayTitle ?? "").includes(AUTOMATION_RUN_NAME_MARKER); +} + +/** + * PURE. How many AUTOMATION-dispatched runs at the head of the history did NOT succeed. + * + * `runs` is newest-first, as the GitHub API returns it. Manual dispatches are dropped entirely rather than + * merely "not resetting" the streak -- excluding is the safer of the two, since a run nobody automated is + * not evidence about the automated path in either direction. + * + * 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. Note this now applies to the FILTERED + * list, so a history of nothing but manual runs correctly reports 0 rather than its full length. */ -export function leadingNonSuccessCount(runs: readonly (string | null | undefined)[]): number { - const firstSuccess = runs.findIndex((conclusion) => conclusion === "success"); - return firstSuccess === -1 ? runs.length : firstSuccess; +export function leadingNonSuccessCount(runs: readonly WorkflowRunSummary[]): number { + const automated = runs.filter(isAutomationDispatched); + const firstSuccess = automated.findIndex((run) => run.conclusion === "success"); + return firstSuccess === -1 ? automated.length : firstSuccess; } /** The tracking issue's title for a workflow. Stable, and derived from the workflow file name, so the @@ -48,11 +109,13 @@ function gh(args: readonly string[]): string { function outageBody(workflow: string, streak: number, threshold: number): string { return [ - `\`${workflow}\` has failed on **${streak} consecutive runs**.`, + `\`${workflow}\` has failed on **${streak} consecutive automation-dispatched 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.", "", + "Manually-dispatched runs are excluded from this count (#10234), so this is not a maintainer's own retries.", + "", "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.`, "", @@ -80,11 +143,18 @@ function main(): void { process.exit(2); } - let conclusions: (string | null)[] = []; + let runs: WorkflowRunSummary[] = []; try { - conclusions = JSON.parse( - gh(["api", `repos/${repo}/actions/workflows/${workflow}/runs?per_page=10&status=completed`, "--jq", "[.workflow_runs[].conclusion]"]), - ) as (string | null)[]; + runs = JSON.parse( + gh([ + "api", + `repos/${repo}/actions/workflows/${workflow}/runs?per_page=10&status=completed`, + "--jq", + // `display_title` is where `run-name:` lands, and it is the only field that recovers dispatch + // provenance -- see the header. Renamed to camelCase here so WorkflowRunSummary stays idiomatic. + "[.workflow_runs[] | {conclusion, event, displayTitle: .display_title}]", + ]), + ) as WorkflowRunSummary[]; } 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. @@ -92,9 +162,9 @@ function main(): void { return; } - const streak = leadingNonSuccessCount(conclusions); + const streak = leadingNonSuccessCount(runs); if (streak < threshold) { - console.log(`${workflow}: ${streak} consecutive failure(s) -- below the ${threshold}-run escalation threshold, treating as transient.`); + console.log(`${workflow}: ${streak} consecutive automated failure(s) -- below the ${threshold}-run escalation threshold, treating as transient.`); return; } diff --git a/test/unit/check-dispatch-provenance-stamped.test.ts b/test/unit/check-dispatch-provenance-stamped.test.ts new file mode 100644 index 0000000000..ec1c9f6460 --- /dev/null +++ b/test/unit/check-dispatch-provenance-stamped.test.ts @@ -0,0 +1,161 @@ +import { readFileSync, readdirSync } from "node:fs"; +import { join } from "node:path"; + +import { describe, expect, it } from "vitest"; + +import { + PROVENANCE_INPUT, + declaresProvenanceInput, + dispatchesMissingFlag, + runNameValue, + stampProblems, +} from "../../scripts/check-dispatch-provenance-stamped"; +import { AUTOMATION_RUN_NAME_MARKER } from "../../scripts/escalate-workflow-outage"; + +// #10234. The provenance stamp is a two-sided string with NO runtime symptom when the sides drift: the +// escalation just reads every automated run as manual and goes quiet forever, while still looking wired. +// These pin the checker that asserts the lockstep, and the last block pins the real workflow files. + +const MARKER = AUTOMATION_RUN_NAME_MARKER; + +const STAMPED_WORKFLOW = `name: Publish Miner Package +on: + workflow_dispatch: + inputs: + ${PROVENANCE_INPUT}: + type: boolean + default: false + +run-name: "Publish Miner Package\${{ inputs.${PROVENANCE_INPUT} && ' ${MARKER}' || '' }}" +`; + +describe("declaresProvenanceInput", () => { + it("detects the input regardless of how deeply it is indented", () => { + expect(declaresProvenanceInput(STAMPED_WORKFLOW)).toBe(true); + expect(declaresProvenanceInput(`on:\n workflow_dispatch:\n inputs:\n ${PROVENANCE_INPUT}:\n`)).toBe(true); + }); + + it("is false for a workflow that does not opt in", () => { + expect(declaresProvenanceInput("name: self-host\non:\n push:\n branches: [main]\n")).toBe(false); + }); + + it("does not match a mere prose mention of the input name", () => { + // The publish workflows discuss the input in comments right above it; a comment must not be read as a + // declaration, or a workflow could pass by talking about the stamp without having one. + expect(declaresProvenanceInput(`# see ${PROVENANCE_INPUT}: below for why\n`)).toBe(false); + }); +}); + +describe("runNameValue", () => { + it("reads the top-level run-name", () => { + expect(runNameValue(STAMPED_WORKFLOW)).toBe(`"Publish Miner Package\${{ inputs.${PROVENANCE_INPUT} && ' ${MARKER}' || '' }}"`); + }); + + it("is undefined when there is none", () => { + expect(runNameValue("name: x\non:\n push:\n")).toBeUndefined(); + }); + + it("only matches at column 0, so a nested key is not mistaken for the workflow's run name", () => { + expect(runNameValue("jobs:\n a:\n run-name: nope\n")).toBeUndefined(); + }); +}); + +describe("stampProblems", () => { + it("is silent for a correctly stamped workflow", () => { + expect(stampProblems(STAMPED_WORKFLOW, MARKER)).toEqual([]); + }); + + it("is silent for a workflow that does not opt in at all", () => { + // selfhost.yml is push-triggered and has no dispatch ambiguity. Demanding a stamp there would be noise. + expect(stampProblems("name: self-host\non:\n push:\n", MARKER)).toEqual([]); + }); + + it("catches an opted-in workflow with no run-name — the stamp reaches display_title via nothing else", () => { + const text = STAMPED_WORKFLOW.replace(/^run-name:.*$/m, ""); + expect(stampProblems(text, MARKER)).toHaveLength(1); + expect(stampProblems(text, MARKER)[0]).toContain("no top-level `run-name:`"); + }); + + it("catches a run-name whose marker drifted from the script's constant", () => { + const text = STAMPED_WORKFLOW.replace(MARKER, "[auto]"); + expect(stampProblems(text, MARKER).join(" ")).toContain(MARKER); + }); + + it("catches an UNCONDITIONAL marker, which would stamp a human's dispatch as automated", () => { + // The subtle inversion: the stamp is present, the checker's first rule passes, and every manual run now + // reads as automated -- restoring the #10171 false alarm the whole change exists to remove. + const text = STAMPED_WORKFLOW.replace(/^run-name:.*$/m, `run-name: "Publish Miner Package ${MARKER}"`); + const problems = stampProblems(text, MARKER); + expect(problems).toHaveLength(1); + expect(problems[0]).toContain(`inputs.${PROVENANCE_INPUT}`); + }); +}); + +describe("dispatchesMissingFlag", () => { + const stamped = new Set(["publish-miner.yml"]); + + it("accepts a dispatch that passes the flag", () => { + expect(dispatchesMissingFlag(`run: gh workflow run publish-miner.yml -f ${PROVENANCE_INPUT}=true\n`, stamped)).toEqual([]); + }); + + it("catches a stamped workflow dispatched without the flag", () => { + expect(dispatchesMissingFlag("run: gh workflow run publish-miner.yml --ref x\n", stamped)).toEqual(["publish-miner.yml"]); + }); + + it("ignores a dispatch of a workflow that has no provenance input", () => { + // orb-beta-release.yml dispatches release-selfhost.yml, which is not part of this mechanism at all. + expect(dispatchesMissingFlag("run: gh workflow run release-selfhost.yml --ref x\n", stamped)).toEqual([]); + }); + + it("requires the flag for a runtime-resolved target, because the reconcile path is the one that matters", () => { + // `gh workflow run "$workflow"` cannot be resolved statically, and it is precisely the call site that + // wraps the escalation. Requiring the flag unconditionally there is the deliberate conservative choice. + expect(dispatchesMissingFlag('run: gh workflow run "$workflow" --repo "$R"\n', stamped)).toEqual(["$workflow"]); + expect(dispatchesMissingFlag(`run: gh workflow run "$workflow" -f ${PROVENANCE_INPUT}=true\n`, stamped)).toEqual([]); + }); + + it("does not read a commented mention of `gh workflow run` as a dispatch", () => { + // These workflow files discuss `gh workflow run` at length in prose. Treating a comment as a call site + // would make the check unsatisfiable. + expect(dispatchesMissingFlag("# a bare `gh workflow run publish-miner.yml` is the human override path\n", stamped)).toEqual([]); + }); + + it("strips quotes from the target before matching", () => { + expect(dispatchesMissingFlag('run: gh workflow run "publish-miner.yml"\n', stamped)).toEqual(["publish-miner.yml"]); + }); + + it("returns nothing for text containing no dispatch at all", () => { + expect(dispatchesMissingFlag("name: x\non:\n push:\n", stamped)).toEqual([]); + }); +}); + +describe("the real workflow files satisfy the invariant (#10234)", () => { + const dir = ".github/workflows"; + const texts = new Map( + readdirSync(dir) + .filter((entry) => entry.endsWith(".yml") || entry.endsWith(".yaml")) + .map((entry) => [entry, readFileSync(join(dir, entry), "utf8")] as const), + ); + const stamped = new Set([...texts].filter(([, text]) => declaresProvenanceInput(text)).map(([file]) => file)); + + it("stamps all five publish workflows and nothing else", () => { + // Pinned as an exact set: a sixth publisher added without a stamp is invisible to the escalation, and a + // stamp appearing on an unrelated workflow means someone copied the block without the dispatch side. + expect([...stamped].sort()).toEqual([ + "publish-contract.yml", + "publish-engine.yml", + "publish-mcp.yml", + "publish-miner.yml", + "publish-ui-kit.yml", + ]); + }); + + it("has no stamp problems and no unflagged dispatch in any workflow", () => { + const failures: string[] = []; + for (const [file, text] of texts) { + for (const problem of stampProblems(text, MARKER)) failures.push(`${file}: ${problem}`); + for (const target of dispatchesMissingFlag(text, stamped)) failures.push(`${file}: ${target} missing the flag`); + } + expect(failures).toEqual([]); + }); +}); diff --git a/test/unit/escalate-workflow-outage.test.ts b/test/unit/escalate-workflow-outage.test.ts index 88eec5d119..3242031689 100644 --- a/test/unit/escalate-workflow-outage.test.ts +++ b/test/unit/escalate-workflow-outage.test.ts @@ -1,6 +1,12 @@ import { describe, expect, it } from "vitest"; -import { leadingNonSuccessCount, outageIssueTitle } from "../../scripts/escalate-workflow-outage"; +import { + AUTOMATION_RUN_NAME_MARKER, + isAutomationDispatched, + leadingNonSuccessCount, + outageIssueTitle, + type WorkflowRunSummary, +} from "../../scripts/escalate-workflow-outage"; // #10146: a post-merge workflow going red blocks nothing and pages no one. // @@ -10,17 +16,35 @@ import { leadingNonSuccessCount, outageIssueTitle } from "../../scripts/escalate // 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. +// +// #10234 added the second axis: WHO started the run. A maintainer's hand retry fails just as deterministically +// as an outage does, so counting it escalated #10171 against a workflow that was never broken. + +/** A run the release automation dispatched: `run-name:` stamped the marker into `display_title`. */ +function automated(conclusion: string | null | undefined): WorkflowRunSummary { + return { conclusion, event: "workflow_dispatch", displayTitle: `Publish Miner Package ${AUTOMATION_RUN_NAME_MARKER}` }; +} + +/** A run a human started with `gh workflow run`: identical on every other field, marker absent. */ +function manual(conclusion: string | null | undefined): WorkflowRunSummary { + return { conclusion, event: "workflow_dispatch", displayTitle: "Publish Miner Package" }; +} + +/** A push-triggered run (selfhost.yml). No dispatch ambiguity exists for it. */ +function pushed(conclusion: string | null | undefined): WorkflowRunSummary { + return { conclusion, event: "push", displayTitle: "self-host" }; +} 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); + expect(leadingNonSuccessCount([automated("failure"), automated("failure"), automated("success"), automated("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); + expect(leadingNonSuccessCount([automated("success"), ...Array(4).fill(automated("failure"))])).toBe(0); }); it("REGRESSION: a window with NO success anywhere reports the whole window, not zero", () => { @@ -29,14 +53,16 @@ describe("leadingNonSuccessCount (#10146)", () => { // 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); + expect(leadingNonSuccessCount([automated("failure"), automated("failure"), automated("failure")])).toBe(3); + expect(leadingNonSuccessCount(Array(10).fill(automated("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); + expect( + leadingNonSuccessCount([automated("cancelled"), automated("timed_out"), automated(null), automated(undefined), automated("failure"), automated("success")]), + ).toBe(5); }); it("is zero for an empty history, so a brand-new workflow never alerts", () => { @@ -44,7 +70,74 @@ describe("leadingNonSuccessCount (#10146)", () => { }); it("does not treat a non-'success' string as success on a prefix match", () => { - expect(leadingNonSuccessCount(["successful", "success"])).toBe(1); + expect(leadingNonSuccessCount([automated("successful"), automated("success")])).toBe(1); + }); +}); + +describe("leadingNonSuccessCount ignores hand dispatches (#10234)", () => { + it("REGRESSION #10171: six manual failures in a row do not escalate", () => { + // The exact shape that filed a bogus outage issue: six consecutive publish-miner.yml failures, every one + // a hand `gh workflow run` against main failing ETARGET on a contract version that was not published + // yet. The workflow was never broken -- the next automated run succeeded with no code change. + expect(leadingNonSuccessCount(Array(6).fill(manual("failure")))).toBe(0); + }); + + it("REGRESSION #10171: manual failures sitting ON TOP of an automated success still do not escalate", () => { + // The full #10171 history, newest-first. Excluding (rather than merely "not resetting") is what makes + // this 0: the automated run underneath them succeeded, so there is no outage at any depth. + const history = [...Array(6).fill(manual("failure")), automated("success"), automated("success")]; + expect(leadingNonSuccessCount(history)).toBe(0); + }); + + it("still escalates a genuine automated streak that manual runs are interleaved with", () => { + // The load-bearing half: filtering must not become a way for a real outage to hide. Three automated + // failures reach the threshold whether or not a maintainer retried by hand in between. + const history = [automated("failure"), manual("success"), automated("failure"), manual("failure"), automated("failure"), automated("success")]; + expect(leadingNonSuccessCount(history)).toBe(3); + }); + + it("a manual SUCCESS cannot mask a real automated outage", () => { + // The dangerous direction of the same rule. If a hand retry succeeded, that says nothing about the + // automated path -- counting it as a streak-breaker would suppress an outage that is still live. + expect(leadingNonSuccessCount([manual("success"), automated("failure"), automated("failure"), automated("failure")])).toBe(3); + }); + + it("a history of nothing but manual runs reports 0, not its full length", () => { + // The no-success-anywhere rule now applies to the FILTERED list. Reading `length` off the unfiltered + // history here would escalate on a workflow the automation has never once run. + expect(leadingNonSuccessCount([manual("failure"), manual("cancelled"), manual("failure")])).toBe(0); + }); + + it("leaves push-triggered workflows (selfhost.yml) counting exactly as before", () => { + // selfhost.yml is the OTHER caller, and it is push-triggered. Narrowing to "stamped dispatches only" + // would have silently switched its alert off -- the regression this change most easily causes. + expect(leadingNonSuccessCount([pushed("failure"), pushed("failure"), pushed("failure")])).toBe(3); + expect(leadingNonSuccessCount([pushed("success"), pushed("failure")])).toBe(0); + }); +}); + +describe("isAutomationDispatched (#10234)", () => { + it("reads the marker out of display_title, which is the only field that carries it", () => { + expect(isAutomationDispatched(automated("failure"))).toBe(true); + expect(isAutomationDispatched(manual("failure"))).toBe(false); + }); + + it("treats every non-dispatch trigger as automation", () => { + // push / schedule / workflow_run cannot be hand-triggered in the way this guards against. + for (const event of ["push", "schedule", "workflow_run", "repository_dispatch"]) { + expect(isAutomationDispatched({ conclusion: "failure", event, displayTitle: "anything" })).toBe(true); + } + }); + + it("treats a missing display_title as manual rather than crashing", () => { + // The API returns display_title for every run today, but an absent one must fail SAFE -- toward "do not + // escalate" -- not throw inside the alerting path, which is deliberately never allowed to fail the caller. + expect(isAutomationDispatched({ conclusion: "failure", event: "workflow_dispatch", displayTitle: null })).toBe(false); + expect(isAutomationDispatched({ conclusion: "failure", event: "workflow_dispatch", displayTitle: undefined })).toBe(false); + }); + + it("a null event is treated as automation, since only a KNOWN workflow_dispatch is ambiguous", () => { + expect(isAutomationDispatched({ conclusion: "failure", event: null, displayTitle: "x" })).toBe(true); }); });