diff --git a/.changeset/workflow-status-function-gate.md b/.changeset/workflow-status-function-gate.md new file mode 100644 index 0000000000..664abb286d --- /dev/null +++ b/.changeset/workflow-status-function-gate.md @@ -0,0 +1,20 @@ +--- +--- + +chore(ci): job 级 `if:` 读 `needs.*.outputs.*` 必须显式点名状态函数,并修掉 publish-smoke 的存量违规 (#5343) + +GitHub 会给任何**不含状态函数**的 `if:` 隐式包一层 `success()`。于是一个本意是「读上游输出值做数据驱动决策」的 job 级条件,悄悄同时携带了一个作者从未写下的状态决策,而「上游挂了」和「上游说不用跑」这两件完全不同的事,到达时是同一个 skipped —— 在任何 checks 列表里都渲染成绿。 + +这个坑本仓已经手工踩过两次,相隔数月:#4900 的发布完整性守卫恰好在它前面那个 job 失败时停止守卫;#4928 在 `ci.yml` 里找到七处 `needs.filter.outputs.*` 闸门,一旦上游出事就自己把自己关掉。规则可静态判定,所以本次把它变成门禁而不是第三次靠人读出来。 + +**新门禁 `scripts/check-workflow-status-functions.mjs`(`pnpm check:workflow-status-functions`,接进 `lint.yml` 的 ESLint job)。** 范围严格限定在 **job 级** `if:` 且读 `needs.*.outputs.*`;step 级条件与 `needs.*.result` 刻意在外(前者的隐式 `success()` 说的是「本 job 前面的步骤挂了就别继续」,通常正是作者要的;后者本来就是状态读,作者已经在推理状态,不是被塞了一层没写的包装)。这条边界是「无需猜意图、也无需豁免名单」的前提,脚本因此**没有**任何 skip-list。 + +判据用**真 YAML 解析**而非 grep `if:` 行,这一步是承重的:grep 分不清「文件坏了」和「没有违规」—— 两者都是零匹配 —— 而且看不见折叠标量。本仓已有两个 workflow 写 `if: >-` 跨行(`merge-queue-triage.yml`、`pr-automation.yml`),#5343 那张靠手工 grep 得出的审计表在那里是盲的。输入缺失一律判红(目录不存在、零个 workflow 文件、YAML 解析失败、没有 `jobs:` 映射、`if:` 不是标量),绝不 `exit 0` 静默放过 —— 即 #4690 反模式的正面。`--self-test` 34 条断言跑真实 `scan()` 路径,`--list` 直接输出审计表(现有 9 处 job 级 `needs.*.outputs.*` 读)。 + +**`publish-smoke.yml` 的唯一存量违规按「显式红,不跑、不猜」修掉。** `pack-smoke` 的行为不变(仍是 resolve 成功且 `run == 'true'` 才跑),但 `success() &&` 现在写了出来;真正新增的是 `resolve-guard` job:`always() && needs.resolve.result == 'failure'` 时判红,并尽力把一条 failure commit status 写回 release PR head。 + +这里**没有**照搬 #4928 的 `!cancelled() && ... != 'false'` 形状,原因是那条「存疑就全跑」在本 workflow 不成立:`ci.yml` 的 filter job 输出带 `|| 'true'` 兜底,而 `resolve` 没有,且 `ref` 也是它算出来的 —— 存疑就跑会 checkout 一个空 ref,花 45 分钟 smoke 掉不知道什么东西,再把结论当作「release candidate 通过」报出去。发布完整性上的**假绿**比没有答案更糟。 + +顺带实测(#5343 留的核实项):`publish-smoke / packed-tarballs` 在 `main` 的分支保护里**不是**必需检查(必需的是 `TypeScript Type Check`、`Build Core`、`Test Core`、`Dogfood Regression Gate` 四项)。所以 resolve 挂掉时那条 status 根本不写的后果不是「PR 被卡住」,而是**它在 release PR 上完全不可见** —— 这正是新增 guard job 回写 status 所填的洞。 + +纯工具链改动,不发版。 diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index a599cfc5c1..822ea99b12 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -301,6 +301,27 @@ jobs: - name: Node-version drift guard run: pnpm check:node-version + # Workflow status-function guard (#5343, from #4900 and #4928). GitHub + # wraps any `if:` that names no status function in an IMPLICIT success(). + # So a job-level condition written to read an upstream job's OUTPUT VALUE + # silently also carries a status decision nobody wrote, and the two facts + # "the upstream job DIED" and "the upstream job said don't run" arrive as + # the same skip — which every checks list renders as green. #4900 found a + # release-integrity guard that stopped guarding exactly when the job before + # it failed; #4928 found seven ci.yml gates that turned themselves off + # whenever something upstream broke. Both were caught by hand, months + # apart, and the rule is statically decidable — so it is a gate now. + # Job-level `if:` reading `needs.*.outputs.*` only: step-level conditions + # and `needs.*.result` are deliberately out of scope (see the script + # header). Real YAML parse, not a grep over `if:` lines — a grep cannot + # distinguish a malformed workflow from a clean one and cannot see the + # folded `if: >-` two of these workflows already use. Runs its own + # --self-test first: the detector can break while every workflow is fine, + # and a scan that quietly stops matching would report OK while reading + # nothing (#4690's family). + - name: Workflow status-function guard + run: pnpm check:workflow-status-functions + # #4248 packaging-hygiene guard. Without a `files` whitelist npm packs the # whole package directory, and 20 of the 49 publishable packages declared # none — so consumers installed TypeScript sources, unit tests and build diff --git a/.github/workflows/publish-smoke.yml b/.github/workflows/publish-smoke.yml index c9c5097e8b..377bd36b63 100644 --- a/.github/workflows/publish-smoke.yml +++ b/.github/workflows/publish-smoke.yml @@ -34,6 +34,14 @@ # commit status on the branch head so it IS visible on the release PR. # Not wired into normal PR CI on purpose: full build + pack + clean install # is far too slow for the inner loop. +# +# That reporting path is also why there is a third job, resolve-guard (#5343): +# every verdict this workflow can produce reaches a human through that commit +# status, so a `resolve` that FAILS writes nothing at all — the release PR then +# looks exactly like "there was nothing to smoke". The status is not a required +# check (branch protection on main requires TypeScript Type Check, Build Core, +# Test Core and Dogfood Regression Gate), so an absent one blocks nothing and is +# simply invisible. resolve-guard turns that silence into an explicit red. name: Publish Smoke @@ -92,10 +100,66 @@ jobs: echo "No open release PR — nothing to smoke" fi + # resolve failing is NOT "nothing to smoke" — it is "we do not know". Say so + # out loud (#5343). Without this job the two outcomes are indistinguishable + # everywhere it matters: pack-smoke is skipped in both, and a skipped job reads + # as green; and since this workflow runs on `workflow_run`, its own red run is + # filed against the default branch, nowhere near the release PR a human is + # about to merge. + # + # The alternative shape — #4928's `!cancelled() && ... != 'false'`, i.e. "when + # in doubt, run everything" — is wrong HERE, and that is why the decision was + # taken per workflow rather than copied. ci.yml's filter job defaults its + # outputs to "run it" (`|| 'true'`); resolve does not, and it also computes the + # `ref` to check out. Running on doubt would check out an EMPTY ref (the + # workflow_run default), spend 45 minutes smoking whatever that resolves to, + # and report the verdict as if it were about the release candidate. A false + # green on release integrity is worse than no answer. + resolve-guard: + name: Resolve target failed — the candidate was NOT smoked + needs: resolve + if: always() && needs.resolve.result == 'failure' + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: read + statuses: write + steps: + - name: Report the gap on the release PR, then fail + env: + GH_TOKEN: ${{ github.token }} + run: | + # Best-effort, and deliberately a SECOND lookup rather than a reuse of + # resolve's outputs: resolve failed, so its outputs are empty — that is + # the whole problem. Same query as resolve's (`--head + # changeset-release/main`); keep the two in step if that convention + # ever changes. Whatever this lookup does, the job still fails below, + # so the fallback can never turn into a silent pass. + release_head=$(gh pr list --repo "$GITHUB_REPOSITORY" \ + --head changeset-release/main --state open \ + --json headRefOid -q '.[0].headRefOid // empty') || release_head='' + if [ -n "$release_head" ]; then + gh api "repos/$GITHUB_REPOSITORY/statuses/$release_head" \ + -f state=failure -f context='publish-smoke / packed-tarballs' \ + -f description='Target resolution failed — the candidate was never smoked' \ + -f target_url="$GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID" \ + || echo "::warning::could not write the commit status on $release_head" + else + echo "::warning::no open changeset-release/main PR found — the failure is reported here only" + fi + echo "::error title=Publish smoke did not run::resolve failed, so the release candidate was never smoked. This is NOT the same as 'nothing to smoke' — re-run the workflow, or check the release PR by hand before merging." + exit 1 + pack-smoke: name: Packed-tarball smoke (release candidate) needs: resolve - if: needs.resolve.outputs.run == 'true' + # success() is spelled out rather than left to GitHub's implicit wrapper + # (#5343): an `if:` naming no status function is silently wrapped in one, so + # "resolve failed" and "resolve said don't run" both arrive here as the same + # skip. The behaviour is unchanged — smoke only a resolve that SUCCEEDED and + # said run — but it is now written down, and resolve-guard above states the + # failure case that this condition deliberately does not cover. + if: success() && needs.resolve.outputs.run == 'true' runs-on: ubuntu-latest timeout-minutes: 45 permissions: diff --git a/package.json b/package.json index 1c719c099f..5fd689e34d 100644 --- a/package.json +++ b/package.json @@ -53,6 +53,7 @@ "check:release-notes": "node scripts/check-release-notes.mjs", "check:release-body": "node scripts/release-github-releases.mjs --self-test", "check:node-version": "node scripts/check-node-version.mjs", + "check:workflow-status-functions": "node scripts/check-workflow-status-functions.mjs --self-test && node scripts/check-workflow-status-functions.mjs", "check:published-files": "node scripts/check-published-files.mjs --self-test && node scripts/check-published-files.mjs", "check:type-check-coverage": "node scripts/check-type-check-coverage.mjs --self-test && node scripts/check-type-check-coverage.mjs", "check:driver-conformance": "node scripts/check-driver-conformance.mjs --self-test && node scripts/check-driver-conformance.mjs", @@ -78,7 +79,8 @@ "tsup": "^8.5.1", "tsx": "^4.23.1", "turbo": "^2.10.7", - "typescript": "^6.0.3" + "typescript": "^6.0.3", + "yaml": "^2.9.0" }, "engines": { "node": ">=22.0.0" diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 7bf02840b0..6c058d4715 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -68,6 +68,9 @@ importers: typescript: specifier: ^6.0.3 version: 6.0.3 + yaml: + specifier: ^2.9.0 + version: 2.9.0 apps/docs: dependencies: diff --git a/scripts/check-workflow-status-functions.mjs b/scripts/check-workflow-status-functions.mjs new file mode 100644 index 0000000000..51ed8b373c --- /dev/null +++ b/scripts/check-workflow-status-functions.mjs @@ -0,0 +1,609 @@ +#!/usr/bin/env node +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. +// +// check-workflow-status-functions -- a JOB-LEVEL `if:` that reads +// `needs..outputs.` must name a status function. +// +// node scripts/check-workflow-status-functions.mjs +// node scripts/check-workflow-status-functions.mjs --self-test # the checker itself +// node scripts/check-workflow-status-functions.mjs --list # the full audit table +// +// ## The rule (#5343, distilled from #4900 and #4928) +// +// Any job-level `if:` that reads `needs.SOMEJOB.outputs.SOMEKEY` must name one +// of `always()`, `!cancelled()`, `success()` or `failure()`. +// +// GitHub wraps every `if:` that contains no status function in an IMPLICIT +// `success()`. So a condition written to read an upstream job's OUTPUT VALUE -- +// a data-driven decision -- silently also carries a status decision the author +// never wrote. The two completely different facts +// +// the upstream job DIED (status) +// the upstream job said "do not run" (data) +// +// are then compressed into the same `skipped`, and a skipped job reads as green +// in every checks list. That is how #4900 shipped a release-integrity guard that +// stopped guarding whenever the job before it failed, and how #4928 found seven +// `needs.filter.outputs.*` gates in ci.yml that turned themselves off exactly +// when something upstream broke. +// +// Wanting `success()` semantics is perfectly legitimate -- pack-smoke in +// publish-smoke.yml genuinely wants them. The rule is that you WRITE IT: +// `success() && needs.x.outputs.y == 'true'`. Declared beats remembered; the +// next reader must not have to carry GitHub's implicit wrapper in their head. +// +// ## Scope, and why it is drawn here +// +// JOB level only, and only `needs.*.outputs.*`. Both halves are deliberate. +// +// - A STEP-level `if:` reading `steps.*.outputs.*` is out. There the implicit +// `success()` means "an earlier step of THIS job failed, so stop", which is +// almost always exactly what the author wants. Scanning those would produce +// a batch of false positives on day one, and a gate born with a long +// exemption list is one step from the #4690 anti-pattern it exists to +// prevent. +// - A STEP-level `if:` reading `needs.*.outputs.*` is out for the same reason: +// the implicit `success()` there is still about the steps of its own job, +// and the job it sits in already had to pass this rule at job level. +// publish-smoke.yml's two `needs.resolve.outputs.report-sha` steps are the +// live specimens -- both correct, neither this gate's business. +// - `needs.*.result` / `needs.*.conclusion` are out. Those ARE status reads; +// an author writing one is already reasoning about upstream status rather +// than being handed a wrapper they did not write. This gate is about the +// silent compression of status into data, not about status expressions. +// +// The scope is what makes the rule statically decidable with no intent-guessing +// and no exemption hatch. There is intentionally NO skip-list: if a job-level +// `if:` ever genuinely needs one, that is a decision to take in the open. +// +// ## Missing input is a failure, never a pass (#4690) +// +// `.github/workflows/` absent, zero workflow files, a file that does not parse, +// a workflow with no `jobs:` map, an `if:` that is not a scalar -- every one of +// them exits non-zero. A checker that cannot read its input has not verified +// anything, and reporting OK for that is the exact defect #4690 recorded: a gate +// that skipped silently and exited 0 read as "no violations" for months. +// +// This is also why the parse is a real YAML parse rather than a grep over `if:` +// lines. A grep cannot tell a malformed workflow from a clean one -- it just +// finds nothing, which is indistinguishable from "no violations" -- and it +// cannot see a folded expression at all. Two workflows in this repo already +// write `if: >-` across several lines (merge-queue-triage.yml, pr-automation.yml); +// the hand-run grep audit in #5343 could not have seen a violation there. + +import { execFileSync } from 'node:child_process'; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, statSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join } from 'node:path'; +import { LineCounter, isMap, isScalar, parseDocument } from 'yaml'; + +const WORKFLOW_DIR = '.github/workflows'; + +/** + * Reads an upstream job's OUTPUT value: `needs..outputs.` in either + * the dot or the index spelling, with GitHub's tolerated whitespace. + * Deliberately stops at `outputs` + accessor, so `needs.build.result` and a bare + * `toJSON(needs)` do not match -- see the scope note in the header. + */ +const READS_NEEDS_OUTPUTS = /\bneeds\s*(?:\.\s*[A-Za-z_][A-Za-z0-9_-]*|\[[^\]]*\])\s*\.\s*outputs\s*(?:\.|\[)/i; + +/** GitHub's four status check functions. Case-insensitive, as the expression language is. */ +const NAMES_STATUS_FUNCTION = /\b(?:always|cancelled|success|failure)\s*\(\s*\)/i; + +/** The four status functions, for the failure prescription. */ +const STATUS_FUNCTIONS = ['success()', '!cancelled()', 'always()', 'failure()']; + +// ── Scanning ──────────────────────────────────────────────────────────────── + +/** + * Every job-level `if:` in the workflow directory, judged. + * + * @param {string} root repository root (or, in --self-test, a fixture root) + * @returns {{ + * violations: { file: string, line: number, job: string, expression: string }[], + * compliant: { file: string, line: number, job: string, expression: string }[], + * problems: string[], + * files: number, + * jobs: number, + * jobIfs: number, + * }} + */ +export function scan(root) { + const violations = []; + const compliant = []; + const problems = []; + let jobs = 0; + let jobIfs = 0; + + const dir = join(root, WORKFLOW_DIR); + if (!existsSync(dir) || !statSync(dir).isDirectory()) { + problems.push(`${WORKFLOW_DIR}/ does not exist -- nothing was scanned, so nothing was verified.`); + return { violations, compliant, problems, files: 0, jobs, jobIfs }; + } + + const files = readdirSync(dir) + .filter((f) => f.endsWith('.yml') || f.endsWith('.yaml')) + .sort(); + if (files.length === 0) { + problems.push(`${WORKFLOW_DIR}/ holds no .yml/.yaml file -- nothing was scanned, so nothing was verified.`); + return { violations, compliant, problems, files: 0, jobs, jobIfs }; + } + + for (const name of files) { + const rel = `${WORKFLOW_DIR}/${name}`; + const source = readFileSync(join(dir, name), 'utf8'); + const lineCounter = new LineCounter(); + let doc; + try { + doc = parseDocument(source, { lineCounter }); + } catch (err) { + problems.push(`${rel}: YAML parse threw -- ${err.message}`); + continue; + } + if (doc.errors.length > 0) { + problems.push(`${rel}: YAML parse error -- ${doc.errors[0].message}`); + continue; + } + + const jobsNode = doc.get('jobs'); + if (!isMap(jobsNode)) { + problems.push(`${rel}: no \`jobs:\` mapping -- a workflow this gate cannot read is a workflow it cannot clear.`); + continue; + } + + for (const pair of jobsNode.items) { + const job = String(pair.key?.value ?? pair.key); + jobs++; + const body = pair.value; + if (!isMap(body)) { + problems.push(`${rel}: job \`${job}\` is not a mapping -- cannot read its \`if:\`.`); + continue; + } + const ifPair = body.items.find((p) => String(p.key?.value) === 'if'); + if (!ifPair) continue; + + const line = lineCounter.linePos(ifPair.key.range[0]).line; + if (!isScalar(ifPair.value) || ifPair.value.value === null || ifPair.value.value === undefined) { + problems.push(`${rel}:${line}: job \`${job}\` has an \`if:\` that is not a scalar expression.`); + continue; + } + jobIfs++; + + const expression = String(ifPair.value.value).replace(/\s+/g, ' ').trim(); + if (!READS_NEEDS_OUTPUTS.test(expression)) continue; + + const entry = { file: rel, line, job, expression }; + if (NAMES_STATUS_FUNCTION.test(expression)) compliant.push(entry); + else violations.push(entry); + } + } + + return { violations, compliant, problems, files: files.length, jobs, jobIfs }; +} + +// ── Reporting ─────────────────────────────────────────────────────────────── + +/** GitHub workflow-command escaping for a message body. */ +function escapeData(value) { + return String(value).replace(/%/g, '%25').replace(/\r/g, '%0D').replace(/\n/g, '%0A'); +} + +/** GitHub workflow-command escaping for an annotation property value. */ +function escapeProperty(value) { + return escapeData(value).replace(/:/g, '%3A').replace(/,/g, '%2C'); +} + +function annotate({ file, line, job, expression }) { + const message = escapeData( + `Job \`${job}\` reads needs.*.outputs.* in its job-level if: without naming a status function, ` + + `so GitHub wraps it in an implicit success() and an upstream FAILURE becomes indistinguishable ` + + `from an upstream "do not run". Write the status function you mean, e.g. \`success() && ${expression}\`.`, + ); + const title = escapeProperty('Job-level if: reads needs.*.outputs.* without a status function'); + return `::error file=${escapeProperty(file)},line=${line},title=${title}::${message}`; +} + +const PRESCRIPTION = ` +Pick the status function you actually mean and write it: + +${STATUS_FUNCTIONS.map((fn) => ` if: \${{ ${fn} && }}`).join('\n')} + + • success() keeps today's behaviour, written down instead of implied. + • !cancelled() runs unless the whole run was cancelled -- the #4928 shape for + a filter job whose outputs default to "run it" (\`|| 'true'\`). + • always() runs even when the run is being cancelled. + • failure() runs only on upstream failure (a guard/reporter job). + +!cancelled() is NOT automatically the right answer. It is only correct when the +upstream outputs still carry a usable value after a failure. publish-smoke.yml's +resolve job is the counter-example #5343 worked through: its \`run\` output has no +\`|| 'true'\` default AND it also computes the \`ref\` to check out, so "when in +doubt run everything" would start a 45-minute job against an empty ref -- smoking +the wrong thing, then reporting green about it. That workflow instead states the +failure explicitly (a guard job that goes red), which is the third option this +rule exists to make visible. + +Scope: job-level \`if:\` only, and only \`needs.*.outputs.*\`. Step-level \`if:\` and +\`needs.*.result\` are deliberately out -- see the header of +scripts/check-workflow-status-functions.mjs.`; + +function repoRoot() { + return execFileSync('git', ['rev-parse', '--show-toplevel'], { encoding: 'utf8' }).trim(); +} + +function summarise({ files, jobs, jobIfs, compliant, violations }) { + const reads = compliant.length + violations.length; + return `scanned ${files} workflow file(s), ${jobs} job(s), ${jobIfs} job-level if: expression(s); ${reads} read needs.*.outputs.*`; +} + +function reportProblems(problems) { + console.error(`check-workflow-status-functions: ${problems.length} input problem(s) -- the scan is NOT a pass\n`); + for (const p of problems) console.error(` • ${p}`); + console.error(` +A gate that could not read its input has verified nothing. Reporting OK here is +the #4690 anti-pattern (a check that silently skipped and exited 0), so this +exits non-zero instead. Fix the input, or fix the checker -- never both quietly.`); +} + +function main() { + const result = scan(repoRoot()); + const { violations, problems } = result; + + if (problems.length > 0) { + reportProblems(problems); + process.exit(1); + } + + if (violations.length === 0) { + console.log(`check-workflow-status-functions: OK (${summarise(result)}, all naming a status function).`); + process.exit(0); + } + + const plural = violations.length === 1 ? 'job-level if:' : 'job-level if: expressions'; + console.error( + `check-workflow-status-functions: ${violations.length} ${plural} read needs.*.outputs.* without naming a status function\n`, + ); + for (const v of violations) { + console.error(` • ${v.file}:${v.line} job \`${v.job}\``); + console.error(` if: ${v.expression}`); + console.log(annotate(v)); + } + console.error(PRESCRIPTION); + process.exit(1); +} + +function list() { + const result = scan(repoRoot()); + if (result.problems.length > 0) { + reportProblems(result.problems); + process.exit(1); + } + const rows = [...result.violations.map((e) => ({ ...e, verdict: 'VIOLATION' })), ...result.compliant.map((e) => ({ ...e, verdict: 'ok' }))].sort( + (a, b) => a.file.localeCompare(b.file) || a.line - b.line, + ); + for (const r of rows) console.log(`${r.verdict.padEnd(9)} ${r.file}:${r.line} ${r.job}\n if: ${r.expression}`); + console.log(`\n${summarise(result)}`); +} + +// ── Self-test ──────────────────────────────────────────────────────────────── +// +// Fixture workflows in a temp dir, run through the SAME scan() main() calls -- +// the check-nul-bytes.mjs convention. scan() reads the directory rather than the +// git index, so a temp dir (no `git init`) is the faithful analogue of that +// script's temp repo: it exercises the real discovery path, not an imitation. + +function selfTest() { + const failures = []; + let checked = 0; + const assert = (cond, msg) => { + checked++; + if (!cond) failures.push(msg); + }; + + const roots = []; + const makeRoot = (files) => { + const dir = mkdtempSync(join(tmpdir(), 'check-workflow-status-fn-')); + roots.push(dir); + for (const [rel, contents] of Object.entries(files)) { + const full = join(dir, rel); + mkdirSync(dirname(full), { recursive: true }); + writeFileSync(full, contents); + } + return dir; + }; + + try { + // ── 1. A violating sample must go red ──────────────────────────────────── + // + // Verbatim shape of publish-smoke.yml:98 as #5343 found it. + const violating = makeRoot({ + '.github/workflows/publish-smoke.yml': `name: Publish Smoke +on: + workflow_run: + workflows: [Release] + types: [completed] +jobs: + resolve: + runs-on: ubuntu-latest + outputs: + run: \${{ steps.target.outputs.run }} + steps: + - id: target + run: echo "run=true" >> "$GITHUB_OUTPUT" + pack-smoke: + needs: resolve + if: needs.resolve.outputs.run == 'true' + runs-on: ubuntu-latest + steps: + - run: echo smoke +`, + }); + const red = scan(violating); + assert(red.problems.length === 0, `a well-formed fixture reports no input problems, got ${red.problems[0]}`); + assert(red.violations.length === 1, `the violating sample is flagged once, got ${red.violations.length}`); + assert(red.violations[0]?.job === 'pack-smoke', `the violating JOB is named, got ${red.violations[0]?.job}`); + assert(red.violations[0]?.line === 16, `the reported line points at the if:, got ${red.violations[0]?.line}`); + assert( + red.violations[0]?.file === '.github/workflows/publish-smoke.yml', + `the file is reported repo-relative, got ${red.violations[0]?.file}`, + ); + // The annotation is the acceptance criterion's own wording -- pin its shape. + const line = annotate(red.violations[0]); + assert( + line.startsWith('::error file=.github/workflows/publish-smoke.yml,line=16,title='), + `the annotation carries file= and line=, got ${line.slice(0, 80)}`, + ); + assert(!/[\r\n]/.test(line.slice(2)), 'the annotation is a single line (raw newlines are escaped)'); + + // ── 2. Compliant samples must stay green ───────────────────────────────── + // + // All four status functions, the #4928 shape, and the negated form -- a rule + // that only recognised `success()` would pass this fixture for the wrong + // reason, so each spelling is asserted separately below. + const green = makeRoot({ + '.github/workflows/ci.yml': `name: CI +on: [push] +jobs: + filter: + runs-on: ubuntu-latest + outputs: + core: \${{ steps.f.outputs.core || 'true' }} + steps: + - id: f + run: echo "core=true" >> "$GITHUB_OUTPUT" + build: + needs: filter + if: \${{ !cancelled() && needs.filter.outputs.core != 'false' }} + runs-on: ubuntu-latest + steps: + - run: echo build + pack: + needs: filter + if: success() && needs.filter.outputs.core == 'true' + runs-on: ubuntu-latest + steps: + - run: echo pack + report: + needs: filter + if: always() && needs.filter.outputs.core == 'true' + runs-on: ubuntu-latest + steps: + - run: echo report + oncall: + needs: filter + if: failure() && needs.filter.outputs.core == 'true' + runs-on: ubuntu-latest + steps: + - run: echo page +`, + }); + const ok = scan(green); + assert(ok.problems.length === 0, `the compliant fixture reports no input problems, got ${ok.problems[0]}`); + assert(ok.violations.length === 0, `the compliant sample is green, got ${ok.violations.length} violation(s)`); + assert(ok.compliant.length === 4, `all four status-function spellings are recognised, got ${ok.compliant.length}`); + for (const fn of ['!cancelled()', 'success()', 'always()', 'failure()']) { + assert( + ok.compliant.some((c) => c.expression.includes(fn)), + `${fn} is recognised as naming a status function`, + ); + } + + // ── 3. Missing input must go red, in all four shapes (#4690) ───────────── + const noDir = makeRoot({ 'README.md': '# no workflows here\n' }); + const missing = scan(noDir); + assert(missing.problems.length === 1, `a missing ${WORKFLOW_DIR}/ is an input problem, got ${missing.problems.length}`); + assert(missing.files === 0 && missing.violations.length === 0, 'a missing directory scans nothing (and says so)'); + + const emptyDir = makeRoot({ '.github/workflows/.gitkeep': '' }); + const empty = scan(emptyDir); + assert(empty.problems.length === 1, `an empty ${WORKFLOW_DIR}/ is an input problem, got ${empty.problems.length}`); + + const malformed = makeRoot({ + '.github/workflows/broken.yml': 'name: Broken\non: [push]\njobs:\n a:\n - this is not\n a mapping: [\n', + }); + const parseFail = scan(malformed); + assert(parseFail.problems.length >= 1, 'a YAML parse failure is an input problem, not a pass'); + assert( + parseFail.problems[0].includes('broken.yml'), + `the unparseable file is named, got ${parseFail.problems[0]}`, + ); + + const jobless = makeRoot({ '.github/workflows/jobless.yml': 'name: Jobless\non: [push]\n' }); + const noJobs = scan(jobless); + assert(noJobs.problems.length === 1, 'a workflow with no jobs: mapping is an input problem'); + + // ── 4. Scope: step level is out, in BOTH spellings ─────────────────────── + // + // This is the boundary the rule was narrowed to on purpose (#5343). Pinning + // it here means a later widening has to be deliberate rather than accidental + // -- and the `needs.*` step case is the one a careless regex would sweep in. + const steps = makeRoot({ + '.github/workflows/steps.yml': `name: Steps +on: [push] +jobs: + resolve: + runs-on: ubuntu-latest + outputs: + report-sha: \${{ steps.t.outputs.sha }} + steps: + - id: t + run: echo "sha=deadbeef" >> "$GITHUB_OUTPUT" + smoke: + needs: resolve + if: success() && needs.resolve.outputs.report-sha != '' + runs-on: ubuntu-latest + steps: + - id: cache + run: echo "cache-hit=true" >> "$GITHUB_OUTPUT" + - name: step reading its own job's step outputs + if: steps.cache.outputs.cache-hit != 'true' + run: echo build + - name: step reading an upstream job's outputs + if: needs.resolve.outputs.report-sha != '' + run: echo report +`, + }); + const stepScope = scan(steps); + assert(stepScope.problems.length === 0, `the step-scope fixture is well-formed, got ${stepScope.problems[0]}`); + assert(stepScope.violations.length === 0, `step-level if: is out of scope, got ${stepScope.violations.length}`); + assert(stepScope.jobIfs === 1, `only the job-level if: is counted, got ${stepScope.jobIfs}`); + + // ── 5. Scope: needs.*.result is a status read, not a data read ─────────── + const resultRead = makeRoot({ + '.github/workflows/guard.yml': `name: Guard +on: [push] +jobs: + resolve: + runs-on: ubuntu-latest + steps: + - run: echo hi + guard: + needs: resolve + if: needs.resolve.result == 'failure' + runs-on: ubuntu-latest + steps: + - run: exit 1 +`, + }); + assert(scan(resultRead).violations.length === 0, 'needs.*.result is outside the rule'); + + // ── 6. A folded expression is read as one expression ───────────────────── + // + // The reason this gate parses YAML instead of grepping `if:` lines: the + // #5343 audit table was built by grep, and a grep over lines cannot see the + // violation below at all -- the `needs.` read and the (absent) status + // function sit on different source lines. Two workflows in this repo + // already write `if: >-`. + const folded = makeRoot({ + '.github/workflows/folded.yml': `name: Folded +on: [push] +jobs: + a: + runs-on: ubuntu-latest + outputs: + go: \${{ steps.s.outputs.go }} + steps: + - id: s + run: echo "go=true" >> "$GITHUB_OUTPUT" + b: + needs: a + if: >- + github.event_name == 'push' && + needs.a.outputs.go == 'true' + runs-on: ubuntu-latest + steps: + - run: echo b +`, + }); + const foldedResult = scan(folded); + assert(foldedResult.violations.length === 1, `a folded violating if: is flagged, got ${foldedResult.violations.length}`); + assert(foldedResult.violations[0]?.line === 13, `the folded if: is located at its key, got ${foldedResult.violations[0]?.line}`); + assert( + !/\n/.test(foldedResult.violations[0]?.expression ?? '\n'), + 'a folded expression is normalised to one line for the report', + ); + // ...and the same expression WITH a status function is green, so the fixture + // proves the parser, not merely that folded scalars are always flagged. + const foldedOk = makeRoot({ + '.github/workflows/folded-ok.yml': readFileSync(join(folded, '.github/workflows/folded.yml'), 'utf8').replace( + " github.event_name == 'push' &&", + " !cancelled() && github.event_name == 'push' &&", + ), + }); + assert(scan(foldedOk).violations.length === 0, 'the same folded expression with !cancelled() is green'); + + // ── 7. Spelling variants the expression language accepts ───────────────── + const variants = makeRoot({ + '.github/workflows/variants.yml': `name: Variants +on: [push] +jobs: + a: + runs-on: ubuntu-latest + steps: + - run: echo hi + index-form: + needs: a + if: \${{ needs['a'].outputs['go'] == 'true' }} + runs-on: ubuntu-latest + steps: + - run: echo b + mixed-case-status: + needs: a + if: \${{ Always() && needs.a.outputs.go == 'true' }} + runs-on: ubuntu-latest + steps: + - run: echo c + wrapped-in-fromjson: + needs: a + if: \${{ contains(fromJSON(needs.a.outputs.list), 'x') }} + runs-on: ubuntu-latest + steps: + - run: echo d +`, + }); + const variantResult = scan(variants); + assert( + variantResult.violations.some((v) => v.job === 'index-form'), + 'the needs[...] index spelling is read as an outputs read', + ); + assert( + variantResult.violations.some((v) => v.job === 'wrapped-in-fromjson'), + 'an outputs read wrapped in fromJSON() is still an outputs read', + ); + assert( + !variantResult.violations.some((v) => v.job === 'mixed-case-status'), + 'the expression language is case-insensitive, so Always() counts', + ); + + // ── 8. The real repository is what this gate actually guards ───────────── + // + // Running scan() over the checkout itself is the reverse direction of the + // fixtures above: fixtures prove the detector can go red, this proves the + // tree it ships with is green. It also fails loudly if the repo's workflow + // directory ever moves out from under the gate. + const real = scan(repoRoot()); + assert(real.problems.length === 0, `the repo's own workflows parse cleanly, got ${real.problems[0]}`); + assert(real.files > 0 && real.jobs > 0, 'the repo scan actually reads workflows'); + assert( + real.compliant.length > 0, + 'the repo has at least one job-level needs.*.outputs.* read -- if this ever hits 0, the gate is guarding nothing', + ); + } finally { + for (const dir of roots) rmSync(dir, { recursive: true, force: true }); + } + + if (failures.length) { + console.error(`✗ check-workflow-status-functions --self-test -- ${failures.length} failure(s)\n`); + for (const f of failures) console.error(` • ${f}`); + process.exit(1); + } + console.log(`✓ check-workflow-status-functions --self-test: ${checked} assertions over temp fixture roots (real scan() path)`); +} + +if (process.argv.includes('--self-test')) { + selfTest(); +} else if (process.argv.includes('--list')) { + list(); +} else { + main(); +}