diff --git a/.github/workflows/branch-naming.yml b/.github/workflows/branch-naming.yml index 59bf1b7d7..9720c0b0b 100644 --- a/.github/workflows/branch-naming.yml +++ b/.github/workflows/branch-naming.yml @@ -11,7 +11,20 @@ concurrency: permissions: {} jobs: + # #3833: no lane runs while the PR has a merge conflict. The workflow-level + # `permissions: {}` above is why the calling job below names both scopes + # explicitly — a calling job's permissions REPLACE the workflow default + # rather than merging with it, so an implicit inherit would hand the + # preflight a token with nothing on it. + preflight: + name: PR mergeability + uses: ./.github/workflows/pr-mergeable-preflight.yml + permissions: + contents: read + pull-requests: read + check-branch: + needs: preflight runs-on: ubuntu-latest timeout-minutes: 1 steps: diff --git a/.github/workflows/changeset-required.yml b/.github/workflows/changeset-required.yml index 6f01a638f..7d6d9886f 100644 --- a/.github/workflows/changeset-required.yml +++ b/.github/workflows/changeset-required.yml @@ -13,7 +13,16 @@ permissions: pull-requests: read jobs: + # #3833: no lane runs while the PR has a merge conflict. + preflight: + name: PR mergeability + uses: ./.github/workflows/pr-mergeable-preflight.yml + permissions: + contents: read + pull-requests: read + changeset-lint: + needs: preflight runs-on: ubuntu-latest steps: - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1 diff --git a/.github/workflows/default-flip-documentation.yml b/.github/workflows/default-flip-documentation.yml index f572ee967..63d9e37ae 100644 --- a/.github/workflows/default-flip-documentation.yml +++ b/.github/workflows/default-flip-documentation.yml @@ -20,7 +20,16 @@ permissions: pull-requests: read jobs: + # #3833: no lane runs while the PR has a merge conflict. + preflight: + name: PR mergeability + uses: ./.github/workflows/pr-mergeable-preflight.yml + permissions: + contents: read + pull-requests: read + default-flip-documentation: + needs: preflight runs-on: ubuntu-latest steps: - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1 diff --git a/.github/workflows/docs-required.yml b/.github/workflows/docs-required.yml index 47762bf2f..5e827bdd6 100644 --- a/.github/workflows/docs-required.yml +++ b/.github/workflows/docs-required.yml @@ -13,7 +13,17 @@ permissions: pull-requests: read jobs: + # #3833: this lane is `npm ci` plus every registered docs-guard test (~210s). + # None of it means anything against a tree that cannot merge. + preflight: + name: PR mergeability + uses: ./.github/workflows/pr-mergeable-preflight.yml + permissions: + contents: read + pull-requests: read + docs-lint: + needs: preflight runs-on: ubuntu-latest # This job supplies the ALREADY-REQUIRED `docs-lint` context # (.github/rulesets/main-protection.json) and runs `npm ci` plus every diff --git a/.github/workflows/install-smoke.yml b/.github/workflows/install-smoke.yml index a487873ef..bc6386a69 100644 --- a/.github/workflows/install-smoke.yml +++ b/.github/workflows/install-smoke.yml @@ -55,7 +55,24 @@ jobs: # --------------------------------------------------------------------------- # Job 1: tarball install (existing canonical path) # --------------------------------------------------------------------------- + # #3833: no compute lane runs while the PR has a merge conflict. + # + # This workflow is ALSO a workflow_call target (release.yml calls it for the + # rc and finalize actions). Two consequences the design depends on: + # 1. On those runs the event is workflow_dispatch, so the script no-ops in + # ~15s with zero API calls and both smoke jobs run exactly as before. + # 2. A nested reusable workflow may only DOWNGRADE its caller's token, so + # release.yml's two calling jobs grant `pull-requests: read` — without + # it the release run fails to start on a permissions escalation. + preflight: + name: PR mergeability + uses: ./.github/workflows/pr-mergeable-preflight.yml + permissions: + contents: read + pull-requests: read + smoke: + needs: preflight runs-on: ${{ matrix.os }} defaults: run: @@ -200,6 +217,7 @@ jobs: # Job 2: unpacked-dir install # --------------------------------------------------------------------------- smoke-unpacked: + needs: preflight runs-on: ubuntu-latest timeout-minutes: 10 diff --git a/.github/workflows/mutation.yml b/.github/workflows/mutation.yml index 5dd836bd0..355606133 100644 --- a/.github/workflows/mutation.yml +++ b/.github/workflows/mutation.yml @@ -36,8 +36,17 @@ jobs: # ── Job 1: detect ───────────────────────────────────────────────────────── # Computes which covered modules changed and emits a matrix for the mutate job. # Intentionally does NOT run npm ci — it only needs git + Node builtins. + # #3833: no compute lane runs while the PR has a merge conflict. + preflight: + name: PR mergeability + uses: ./.github/workflows/pr-mergeable-preflight.yml + permissions: + contents: read + pull-requests: read + detect: name: Detect changed covered modules + needs: preflight runs-on: ubuntu-latest outputs: has_work: ${{ steps.matrix.outputs.has_work }} @@ -127,16 +136,24 @@ jobs: # stryker.config.mjs (which reads process.env.MUTATION_BREAK for the # break threshold). See ADR-456 / issue #1187. # Note: Stryker 9.x has no --break CLI flag; the threshold is config-only. + # Every matrix value is bound here and referenced as a shell variable + # below. CONTRIBUTING.md's final rule forbids `${{ }}` inside a `run:` + # block outright — binding via `env:` is the sanctioned form. Fixed as + # part of #3833 rather than left in place for the next reader, per the + # repo's no-defer rule. env: NODE_OPTIONS: '--max-old-space-size=4096' MUTATION_TEST_CMD: node --test ${{ matrix.tests }} MUTATION_BREAK: ${{ matrix.minScore }} + MODULE_NAME: ${{ matrix.name }} + MUTATE_GLOB: ${{ matrix.mutate }} + MODULE_TESTS: ${{ matrix.tests }} run: | - echo "Module: ${{ matrix.name }}" - echo "Mutate: ${{ matrix.mutate }}" - echo "Tests: ${{ matrix.tests }}" - echo "MinScore: ${{ matrix.minScore }}" - npx stryker run --incremental --mutate "${{ matrix.mutate }}" + echo "Module: ${MODULE_NAME}" + echo "Mutate: ${MUTATE_GLOB}" + echo "Tests: ${MODULE_TESTS}" + echo "MinScore: ${MUTATION_BREAK}" + npx stryker run --incremental --mutate "${MUTATE_GLOB}" - name: Upload mutation report — ${{ matrix.name }} uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 @@ -157,7 +174,7 @@ jobs: # change. The per-module shards report as "Stryker ()". mutation-gate: name: Stryker mutation score (changed files only) - needs: [detect, mutate] + needs: [detect, mutate, preflight] # has_work=false is rendered by SKIPPING this job entirely (job-level `if:`), not by an # early `exit 0` inside the step — a step that runs and exits 0 always reports `success`, # indistinguishable in the UI from "evaluated and passed". A job-level-skipped job reports @@ -167,13 +184,25 @@ jobs: runs-on: ubuntu-latest steps: - name: Evaluate gate + # Values arrive through `env:`, never interpolated into the script body — + # CONTRIBUTING.md's final rule. They were inline `${{ }}` until #3833 + # touched this job; fixed here rather than left for the next reader. + env: + PREFLIGHT: ${{ needs.preflight.result }} + DETECT: ${{ needs.detect.result }} + MUTATE: ${{ needs.mutate.result }} run: | - DETECT="${{ needs.detect.result }}" - MUTATE="${{ needs.mutate.result }}" - + echo "preflight result : ${PREFLIGHT}" echo "detect result : ${DETECT}" echo "mutate result : ${MUTATE}" + # #3833: report the real cause first. Without this arm a conflicted PR + # skips `detect` and the gate blames scope detection for a conflict. + if [ "${PREFLIGHT}" != "success" ]; then + echo "FAIL: PR mergeability preflight did not pass (${PREFLIGHT}) — resolve the merge conflict and push again" + exit 1 + fi + if [ "${DETECT}" != "success" ]; then echo "FAIL: detect job did not succeed (${DETECT})" exit 1 diff --git a/.github/workflows/pr-mergeable-preflight.yml b/.github/workflows/pr-mergeable-preflight.yml new file mode 100644 index 000000000..065252a41 --- /dev/null +++ b/.github/workflows/pr-mergeable-preflight.yml @@ -0,0 +1,131 @@ +name: PR Mergeability Preflight + +# Reusable gate (#3833). A pull request with a merge conflict must run NO CI +# until the conflict is resolved. +# +# Before this existed, the only conflict detector was scripts/ci-rebase-check.cjs, +# which runs INSIDE each job after checkout — so a conflicted PR provisioned the +# whole 12-cell test matrix, both install-smoke jobs, the Stryker shards and the +# docs-guard lane, and every one of them failed with the same annotation. The +# lanes with no rebase check at all ran to completion against a tree that cannot +# merge, producing verdicts about a state that will never exist. +# +# ci-rebase-check.cjs is deliberately UNCHANGED and remains the authoritative +# in-job backstop, including the #2472 base-sha pin. This workflow is an +# early-exit optimization layered above it, never a replacement: every path +# except an explicit `mergeable: false` fails OPEN. +# +# Callers add one job: +# +# preflight: +# uses: ./.github/workflows/pr-mergeable-preflight.yml +# permissions: +# contents: read +# pull-requests: read +# +# and `needs: preflight` on each compute job. A calling job's `permissions` +# REPLACES the workflow default rather than merging with it, so both scopes must +# be named — branch-naming.yml's workflow default is `{}`, which would otherwise +# starve the API read. +# +# This job deliberately applies NO label. #3833 originally proposed adding and +# removing `needs-review: merge-conflict` here; that was dropped during +# implementation for two reasons. Eight caller workflows each invoke this +# preflight, so eight jobs would race to add-or-remove one label on every PR +# event — label thrash and API contention. And it would force +# `pull-requests: write` into eight lanes that today hold `contents: read` +# (and one that holds `permissions: {}`), widening the token everywhere to +# write a signal the red check already carries. `needs-review: merge-conflict` +# remains a maintainer triage label applied during PR sweeps. +# +# The job is NOT guarded with `if: github.event_name == 'pull_request'`. A +# SKIPPED dependency skips its dependents exactly as a failed one does, so that +# guard would skip the entire pipeline on every push to `next` and on every +# release.yml run that calls install-smoke.yml. Instead the job always runs and +# scripts/ci-pr-mergeability.cjs no-ops (zero API calls, exit 0) whenever the +# event is not `pull_request`. + +on: + workflow_call: + outputs: + mergeable: + description: >- + 'false' only when GitHub reported an explicit merge conflict. + 'true' for a clean PR, for an indeterminate read, and for any + non-pull_request event — the gate is fail-open by design. + value: ${{ jobs.mergeability.outputs.mergeable }} + verdict: + description: >- + One of MERGEABLE | CONFLICTED | INDETERMINATE | SKIPPED_NOT_A_PR. + value: ${{ jobs.mergeability.outputs.verdict }} + +# Deliberately NO `concurrency:` block. A group name matching a caller's, with +# cancel-in-progress on both sides, cancels the running CALLER. + +permissions: + contents: read + pull-requests: read + +jobs: + mergeability: + name: mergeability + runs-on: ubuntu-latest + # The whole job is one sparse checkout plus at most five bounded API reads. + # The script's own backoff across the full budget stays under 60s. + timeout-minutes: 3 + outputs: + mergeable: ${{ steps.check.outputs.mergeable }} + verdict: ${{ steps.check.outputs.verdict }} + steps: + - name: Check out the mergeability script from the base branch + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + # Pinned to the BASE sha, never the default ref, for two reasons. + # + # 1. Correctness — and this is the trap the whole design turns on. + # For a pull_request event GitHub sets the default ref to + # refs/pull//merge, which exists only "for open, MERGEABLE pull + # requests". On a conflicted PR that ref is stale or absent, so a + # default checkout fails on precisely the pull requests this job + # exists to diagnose. + # + # 2. Security — the script that decides the gate then comes from a + # trusted commit. No PR-supplied code is fetched or executed here, + # which matches the base-context posture pr-template-format.yml + # already uses ("Check out policy from base branch"). + # + # Empty on a non-pull_request event, where actions/checkout falls back + # to the triggering ref. That is fine: the script no-ops before it + # reads anything. + ref: ${{ github.event.pull_request.base.sha }} + fetch-depth: 1 + sparse-checkout: | + scripts + persist-credentials: false + + - name: Resolve pull request mergeability + id: check + env: + # Every value arrives through env. CONTRIBUTING.md forbids `${{ }}` + # inside a `run:` block. + GITHUB_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + # Bootstrap arm. The checkout above is of the BASE sha, so this step + # runs the script as it exists on the base branch — which is the whole + # point (trusted code, and a usable ref on a conflicted PR), but it + # means the script is absent on the one pull request that INTRODUCES + # it, and on any PR branched from a base predating it. Absent is not + # "conflicted": it is one more thing we cannot determine, so it takes + # the same fail-open path as an unknown `mergeable`, reusing the + # INDETERMINATE atom with its exact documented meaning. Self-healing — + # once the script is on the base branch this arm never fires again. + if [ ! -f scripts/ci-pr-mergeability.cjs ]; then + echo "::warning::scripts/ci-pr-mergeability.cjs is not present at the base sha; skipping the mergeability preflight (fail-open). Expected on the PR that introduces it, or on a branch whose base predates it. scripts/ci-rebase-check.cjs remains the in-job backstop." + { + echo "verdict=INDETERMINATE" + echo "mergeable=true" + } >> "$GITHUB_OUTPUT" + exit 0 + fi + node scripts/ci-pr-mergeability.cjs diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index a90efe9bb..3e120e59b 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -340,6 +340,12 @@ jobs: if: inputs.action == 'rc' permissions: contents: read + # #3833: install-smoke.yml now calls the mergeability preflight, which + # requests pull-requests: read. A nested reusable workflow may only + # DOWNGRADE its caller's token, so without this the whole release run + # fails to start on a permissions escalation. On a release the preflight + # itself no-ops (the event is not pull_request) and never uses the scope. + pull-requests: read uses: ./.github/workflows/install-smoke.yml with: ref: ${{ needs.validate-version.outputs.branch }} @@ -528,6 +534,9 @@ jobs: if: inputs.action == 'finalize' permissions: contents: read + # #3833: see install-smoke-rc — the nested preflight requests this scope, + # and a nested reusable workflow can only downgrade, never elevate. + pull-requests: read uses: ./.github/workflows/install-smoke.yml with: ref: ${{ needs.validate-version.outputs.branch }} diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index d25c3c9e4..f44e31041 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -26,7 +26,17 @@ permissions: contents: read jobs: + # #3833: no compute lane runs while the PR has a merge conflict. No-ops on + # push and workflow_dispatch, so this dependency is unconditional. + preflight: + name: PR mergeability + uses: ./.github/workflows/pr-mergeable-preflight.yml + permissions: + contents: read + pull-requests: read + security: + needs: preflight runs-on: ubuntu-latest # 30m (was 10m): the base64 scan is O(changed-files); a very large diff # (e.g. the #604 repo-wide rename, ~800 changed files) can exceed 10m. The diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a868d9de8..b6d732cbf 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -21,6 +21,21 @@ permissions: contents: read jobs: + # #3833: a conflicted PR must provision no compute runner. The gate fails OPEN + # on everything except an explicit `mergeable: false`, and it no-ops (exit 0, + # zero API calls) on push and workflow_dispatch — so this job is safe to make + # an unconditional dependency of the lanes below. + # + # `changes` is deliberately NOT gated: it is a 2-minute scope classifier that + # runs in parallel with this job, and serializing it behind the preflight + # would add latency to every healthy PR for no saving. + preflight: + name: PR mergeability + uses: ./.github/workflows/pr-mergeable-preflight.yml + permissions: + contents: read + pull-requests: read + changes: name: Detect test scope runs-on: ubuntu-latest @@ -85,6 +100,7 @@ jobs: # Static lint: test-suite and PR-check contracts. # Runs once, not per matrix lane. lint-tests: + needs: preflight runs-on: ubuntu-latest timeout-minutes: 5 steps: @@ -113,7 +129,7 @@ jobs: test: name: test (${{ matrix.os }}, ${{ matrix.node-version }}${{ matrix.shard && format(', shard {0}', matrix.shard) || '' }}) - needs: changes + needs: [changes, preflight] if: needs.changes.outputs.product_changed == 'true' runs-on: ${{ matrix.os }} # #2952: the `scope: full` lane is sharded three ways (see the matrix @@ -335,7 +351,7 @@ jobs: test-inert: name: test (inert CI) - needs: changes + needs: [changes, preflight] if: needs.changes.outputs.code_changed == 'true' && needs.changes.outputs.product_changed != 'true' runs-on: ubuntu-latest timeout-minutes: 15 @@ -389,7 +405,7 @@ jobs: test-full: name: full test (${{ matrix.os }}, ${{ matrix.node-version }}, shard ${{ matrix.shard }}/3) - needs: changes + needs: [changes, preflight] if: needs.changes.outputs.code_changed == 'true' && needs.changes.outputs.full_matrix == 'true' runs-on: ${{ matrix.os }} defaults: @@ -554,7 +570,7 @@ jobs: # include/exclude globs changes, only where they are evaluated. coverage-gate: name: Coverage gate (merged shards) - needs: [changes, test] + needs: [changes, test, preflight] if: needs.changes.outputs.product_changed == 'true' && needs.test.result == 'success' runs-on: ubuntu-latest timeout-minutes: 15 @@ -627,7 +643,7 @@ jobs: # to walk on a docs-only change. qa-loop-walk: name: QA loop walk (smell ratchet) - needs: changes + needs: [changes, preflight] if: needs.changes.outputs.product_changed == 'true' runs-on: ubuntu-latest timeout-minutes: 15 @@ -686,6 +702,7 @@ jobs: required-tests: name: Required tests needs: + - preflight - changes - lint-tests - test @@ -699,6 +716,7 @@ jobs: steps: - name: Summarize required test gate env: + PREFLIGHT_RESULT: ${{ needs.preflight.result }} CODE_CHANGED: ${{ needs.changes.outputs.code_changed }} PRODUCT_CHANGED: ${{ needs.changes.outputs.product_changed }} CHANGES_RESULT: ${{ needs.changes.result }} @@ -710,6 +728,7 @@ jobs: QA_LOOP_WALK_RESULT: ${{ needs.qa-loop-walk.result }} run: | set -euo pipefail + echo "preflight=$PREFLIGHT_RESULT" echo "code_changed=$CODE_CHANGED" echo "product_changed=$PRODUCT_CHANGED" echo "changes=$CHANGES_RESULT" @@ -720,6 +739,18 @@ jobs: echo "coverage-gate=$COVERAGE_GATE_RESULT" echo "qa-loop-walk=$QA_LOOP_WALK_RESULT" + # #3833: the preflight arm comes FIRST and reports its own cause. + # Without it a conflicted PR skips `changes` too, and this gate would + # blame scope detection for a merge conflict. It also matters that + # this job stays `if: always()` and exits NON-ZERO here: a required + # status check that merely goes absent does not block a merge (see + # the mutation-gate note in mutation.yml, #3319), so the required + # `Required tests` context has to actually report red. + if [ "$PREFLIGHT_RESULT" != "success" ]; then + echo "::error::PR mergeability preflight did not pass ($PREFLIGHT_RESULT) — resolve the merge conflict and push again; no test lane runs until then" + exit 1 + fi + if [ "$CHANGES_RESULT" != "success" ]; then echo "::error::test scope detection did not pass" exit 1 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f583cc9ba..a8adc8143 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1252,6 +1252,23 @@ does not recognize `git stash`, `git rm --cached`, `git restore --staged`, or `git update-index --add`, any of which can also move `.planning/` content into a state a later commit picks up. +### A conflicted PR runs no CI + +Every `pull_request` compute lane waits on one shared gate, `PR mergeability`. +If GitHub reports your PR as having a merge conflict, **nothing runs** — no test +matrix, no install smoke, no mutation shards, no docs or changeset lint — until +you resolve it. The check annotates the base branch and the fix: + +```bash +git fetch origin && git rebase origin/next && git push --force-with-lease +``` + +The gate fails **open**: if GitHub cannot tell us whether the PR is mergeable, +the pipeline runs exactly as it did before, and the per-job +`scripts/ci-rebase-check.cjs` still catches the conflict. Full reference, +including which lanes are deliberately *not* gated, is in +[docs/TESTING-SUITES.md → The mergeability preflight](docs/TESTING-SUITES.md#the-mergeability-preflight). + ### CI Test Quality Checks The following checks run on every PR in addition to the test suite: diff --git a/docs/TESTING-SUITES.md b/docs/TESTING-SUITES.md index 4686c3ca3..be82fd979 100644 --- a/docs/TESTING-SUITES.md +++ b/docs/TESTING-SUITES.md @@ -350,6 +350,99 @@ Reported paths are labelled `CREATED`, `MODIFIED`, `DELETED`, or `UNVERIFIED`. `UNVERIFIED` means a scan bound was hit and the path could not be attested either way — it is never the same as clean. +## The mergeability preflight + +Before any of the matrix below is provisioned, every `pull_request` compute lane +waits on one shared gate: **`PR mergeability`**, the reusable workflow +`.github/workflows/pr-mergeable-preflight.yml`. **A pull request with a merge +conflict runs no CI at all until the conflict is resolved** (#3833). + +### Reference + +| Verdict | When | Job result | Effect on the pipeline | +|---|---|---|---| +| `MERGEABLE` | GitHub reported `mergeable: true` | success | everything runs as normal | +| `CONFLICTED` | GitHub reported `mergeable: false` | **failure** | every gated job is skipped; `Required tests` and `Stryker mutation score` report **red** | +| `INDETERMINATE` | mergeability still unknown after the retry budget, or the API read failed | success **(fails open)** | everything runs as normal; a `::warning::` is emitted | +| `SKIPPED_NOT_A_PR` | the event is not `pull_request` (push, `workflow_dispatch`, a `release.yml` call into `install-smoke.yml`) | success | everything runs as normal; **zero** API calls | +| `INDETERMINATE` (bootstrap) | `scripts/ci-pr-mergeability.cjs` is absent at the base sha | success **(fails open)** | everything runs as normal; a `::warning::` names the cause | + +The bootstrap row is a consequence of the checkout being pinned to the base sha: +the preflight runs the script **as it exists on the base branch**, so the script +is absent on the pull request that introduces it, and on any branch whose base +predates it. Absent is not "conflicted" — it is one more thing the gate cannot +determine, so it takes the same fail-open path. The arm is self-healing and +never fires again once the script is on the base branch; a test pins it in place +so a future reader does not mistake it for dead code. + +The verdict comes from `scripts/ci-pr-mergeability.cjs`, which polls +`GET /repos/{owner}/{repo}/pulls/{number}` until `mergeable` is non-null. +GitHub computes mergeability in an **asynchronous background job** and returns +`null` while it is in progress, so a single cold read is never authoritative — +the poll is required, not defensive. + +Two properties are load-bearing and easy to break: + +- **Only `mergeable === true` / `=== false` decides the verdict.** `null` is + falsy, so a truthiness test (`if (!mergeable)`) would classify every cold read + as a conflict and red every PR in the repo. +- **`mergeable_state` never decides the verdict.** Its `blocked` value means + "required checks have not passed", which is true *while this very job is + running* — gating on it self-deadlocks the pipeline. It is read for the + failure message only. + +Gated: `test.yml` (`lint-tests`, `test`, `test-inert`, `test-full`, +`coverage-gate`, `qa-loop-walk`, `required-tests`), `install-smoke.yml`, +`mutation.yml`, `security-scan.yml`, `docs-required.yml`, +`changeset-required.yml`, `default-flip-documentation.yml`, `branch-naming.yml`. + +**Deliberately not gated:** the `pull_request_target` policy and security lanes — +auto-close-unsolicited-PRs, close-draft-PRs, the target/title/template +validators, issue-link, and unauthorized-approval dismissal. Gating those would +let a conflicted drive-by PR evade auto-close, so they keep running on a +conflicted PR and a test asserts they are never wired to the preflight. + +### What it deliberately does not do + +**It applies no label.** The gate does not add or remove `needs-review: merge-conflict`. +Eight caller workflows each invoke the preflight, so eight jobs would race to +add-or-remove one label on every PR event, and it would force +`pull-requests: write` into eight lanes that today hold `contents: read`. +`needs-review: merge-conflict` stays a maintainer triage label applied during PR +sweeps; the red check and its annotation are the machine signal. + +**It changes no branch protection.** `.github/rulesets/main-protection.json` is +untouched and needs no new required context — GitHub natively refuses to merge a +pull request with conflicts, so the ruleset already blocks it. (Adding the check +as *required* before the workflow exists on the base branch would deadlock every +open PR until it landed.) + +### What this does not replace + +`scripts/ci-rebase-check.cjs` is unchanged and still runs inside each matrix job, +including its #2472 base-sha pin. The preflight is an early-exit optimization on +GitHub's asynchronously-computed view; the in-job merge remains authoritative and +covers the case where the base advances mid-run. **The gate is never a safety +property** — every path except an explicit `mergeable: false` fails open, so an +API outage simply restores the pre-#3833 behavior. + +### If your PR shows a red `PR mergeability` check + +The annotation names the base branch and the remedy. There is one step: + +```bash +git fetch origin && git rebase origin/next && git push --force-with-lease +``` + +Resolve the conflicts the rebase reports, then push. The next `synchronize` +event re-runs the preflight and the full pipeline comes back. (Because the +sequence is a single command, this has no separate how-to page — see +[CONTRIBUTING.md → Where Do I Open My PR?](../CONTRIBUTING.md#where-do-i-open-my-pr-branching-model) +for the branching model that makes the rebase necessary in the first place.) + +Note the interaction with the sha-bound pass marker: a rebase changes your HEAD +sha, which invalidates any prior remote-runner verification. Rebase *last*. + ## CI matrix The `Tests` workflow runs every PR through a scoped gate generated by diff --git a/scripts/ci-pr-mergeability.cjs b/scripts/ci-pr-mergeability.cjs new file mode 100644 index 000000000..ba4316134 --- /dev/null +++ b/scripts/ci-pr-mergeability.cjs @@ -0,0 +1,262 @@ +#!/usr/bin/env node +'use strict'; + +// ci-pr-mergeability.cjs — #3833 PR mergeability preflight. +// +// Risk asymmetry drives this design: a false positive (a clean PR wrongly +// classified CONFLICTED) skips every gated compute lane in all eight gated +// workflows on EVERY PR — a repo-wide outage. A false negative (a real +// conflict missed) is today's behavior; the per-job scripts/ci-rebase-check.cjs +// backstop still catches it there. So the weight is entirely on the +// false-positive surface: the single predicate `mergeable === false`, and +// every falsy-but-not-false value around it. +// +// GitHub's `mergeable` field on GET /repos/:owner/:repo/pulls/:number is a +// documented tri-state: `true`, `false`, or `null` while a background +// computation is still running. `null` is falsy — `if (!mergeable)` would +// misclassify "still computing" as "conflicted" and would have red every PR +// in the repo the moment this job started running. Only strict `=== false` +// counts as a conflict. +// +// `mergeable_state` is read only for the human-facing annotation, never for +// the verdict: its `blocked` value means "required checks have not passed", +// which is true for every PR while THIS preflight job itself is one of the +// required checks still running — classifying on it would self-deadlock the +// pipeline (the gate can never go green because it is itself ungreen). +// +// Because `mergeable` starts null and GitHub computes it asynchronously, this +// polls with linear backoff up to a small budget, then fails OPEN +// (INDETERMINATE => exit 0) rather than blocking the world on an API that +// hasn't finished thinking. + +const { ExitError, runMain } = require('./lib/cli-exit.cjs'); + +const VERDICT = Object.freeze({ + MERGEABLE: 'MERGEABLE', + CONFLICTED: 'CONFLICTED', + INDETERMINATE: 'INDETERMINATE', + SKIPPED_NOT_A_PR: 'SKIPPED_NOT_A_PR', +}); + +const MAX_ATTEMPTS = 5; +const BASE_DELAY_MS = 2000; + +/** + * Pure, total classifier: payload -> VERDICT atom. Never throws. + * Deliberately reads ONLY `mergeable`, and only with strict equality — see + * the header comment for why `mergeable_state` and truthiness are excluded. + */ +function classifyMergeability(payload) { + if (payload === null || typeof payload !== 'object') return VERDICT.INDETERMINATE; + if (payload.mergeable === true) return VERDICT.MERGEABLE; + if (payload.mergeable === false) return VERDICT.CONFLICTED; + return VERDICT.INDETERMINATE; +} + +/** + * Linear backoff: attempt 1..N-1 gives 1x..(N-1)x baseMs. Non-positive/NaN + * attempts clamp to one base interval rather than producing 0 or NaN delays. + */ +function nextDelayMs(attempt, { baseMs = BASE_DELAY_MS } = {}) { + const n = Number.isFinite(attempt) && attempt > 0 ? Math.floor(attempt) : 1; + return baseMs * n; +} + +/** + * Dependency-injected poll loop. Resolves as soon as GitHub reports a + * definite mergeable/conflicted state, or gives up after maxAttempts and + * reports INDETERMINATE (fail-open). + * + * @returns {Promise<{verdict:string, attempts:number, payload:*, reason:string}>} + */ +async function resolveMergeability({ + fetchPr, + sleep, + eventName, + prNumber, + maxAttempts = MAX_ATTEMPTS, + baseDelayMs = BASE_DELAY_MS, +} = {}) { + if (eventName !== 'pull_request') { + return { verdict: VERDICT.SKIPPED_NOT_A_PR, attempts: 0, payload: null, reason: 'not-a-pull-request' }; + } + if (!Number.isInteger(prNumber) || prNumber <= 0) { + return { verdict: VERDICT.INDETERMINATE, attempts: 0, payload: null, reason: 'no-pr-number' }; + } + + let attempts = 0; + let lastPayload = null; + let lastError = null; + + for (let i = 1; i <= maxAttempts; i++) { + attempts = i; + let payload; + try { + payload = await fetchPr(prNumber); + lastPayload = payload; + lastError = null; + } catch (err) { + lastError = err; + payload = null; + } + + const verdict = classifyMergeability(payload); + if (verdict === VERDICT.MERGEABLE || verdict === VERDICT.CONFLICTED) { + return { verdict, attempts, payload, reason: 'resolved' }; + } + + if (i < maxAttempts) { + await sleep(nextDelayMs(i, { baseMs: baseDelayMs })); + } + } + + return { + verdict: VERDICT.INDETERMINATE, + attempts, + payload: lastPayload, + reason: lastError ? 'fetch-failed' : 'mergeability-not-computed', + }; +} + +function usage() { + return [ + 'Usage:', + ' node scripts/ci-pr-mergeability.cjs', + '', + 'CI-only preflight: polls the GitHub API for a PR\'s mergeable state and', + 'fails the job (exit 1) only on a definite conflict. Everything else', + '(unknown, unreadable, not a PR) fails open (exit 0).', + '', + 'Environment variables read:', + ' GITHUB_EVENT_NAME workflow trigger event; only "pull_request" is polled', + ' GITHUB_REPOSITORY owner/repo', + ' GITHUB_TOKEN optional bearer token for the API read', + ' GITHUB_BASE_REF PR base branch name (default: next)', + ' GITHUB_API_URL GitHub API base URL (default: https://api.github.com)', + ' PR_NUMBER the pull request number', + ' GITHUB_OUTPUT path to append verdict=/mergeable= step outputs to', + ' GITHUB_STEP_SUMMARY path to append a human-readable summary to', + ' GSD_PR_MERGEABILITY_BASE_DELAY_MS optional numeric override of the base backoff, for tests', + ].join('\n'); +} + +function parseArgs(argv) { + for (const arg of argv) { + if (arg === '--help' || arg === '-h') { + process.stdout.write(`${usage()}\n`); + throw new ExitError(0); + } + throw new Error(`unknown argument: ${arg}`); + } +} + +function writeOutput(lines) { + const outputPath = process.env.GITHUB_OUTPUT; + if (typeof outputPath !== 'string' || outputPath === '') return; + try { + const fs = require('node:fs'); + fs.appendFileSync(outputPath, `${lines.join('\n')}\n`); + } catch (err) { + // A failure while REPORTING the verdict must never invert the gate. + process.stderr.write(`::warning::failed to write GITHUB_OUTPUT: ${err.message}\n`); + } +} + +function writeSummary(text) { + const summaryPath = process.env.GITHUB_STEP_SUMMARY; + if (typeof summaryPath !== 'string' || summaryPath === '') return; + try { + const fs = require('node:fs'); + fs.appendFileSync(summaryPath, `${text}\n`); + } catch (err) { + process.stderr.write(`::warning::failed to write GITHUB_STEP_SUMMARY: ${err.message}\n`); + } +} + +// `argv` defaults to real CLI argv but is a parameter so tests can call +// main() in-process (e.g. main([])) without inheriting the test runner's own +// argv, which would otherwise trip parseArgs's "unknown argument" branch. +async function main(argv = process.argv.slice(2)) { + parseArgs(argv); + + const eventName = process.env.GITHUB_EVENT_NAME; + const repo = process.env.GITHUB_REPOSITORY || ''; + const token = process.env.GITHUB_TOKEN || ''; + const baseBranch = process.env.GITHUB_BASE_REF || 'next'; + const apiBase = process.env.GITHUB_API_URL || 'https://api.github.com'; + const prNumberRaw = process.env.PR_NUMBER; + const prNumber = Number.parseInt(prNumberRaw, 10); + + const overrideDelay = Number(process.env.GSD_PR_MERGEABILITY_BASE_DELAY_MS); + const baseDelayMs = Number.isFinite(overrideDelay) && overrideDelay >= 0 ? overrideDelay : BASE_DELAY_MS; + + const fetchPr = async (number) => { + const url = `${apiBase}/repos/${repo}/pulls/${number}`; + const headers = { + accept: 'application/vnd.github+json', + 'x-github-api-version': '2022-11-28', + 'user-agent': 'gsd-core-ci-pr-mergeability', + }; + if (token) headers.authorization = `Bearer ${token}`; + const response = await fetch(url, { headers, signal: AbortSignal.timeout(10000) }); + if (!response.ok) { + throw new Error(`GitHub API responded ${response.status}`); + } + const text = await response.text(); + try { + return JSON.parse(text); + } catch { + return null; + } + }; + + const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); + + const result = await resolveMergeability({ + fetchPr, + sleep, + eventName, + prNumber: Number.isNaN(prNumber) ? NaN : prNumber, + maxAttempts: MAX_ATTEMPTS, + baseDelayMs, + }); + + const mergeable = result.verdict === VERDICT.CONFLICTED ? 'false' : 'true'; + writeOutput([`verdict=${result.verdict}`, `mergeable=${mergeable}`]); + writeSummary(`PR mergeability preflight: ${result.verdict}`); + + if (result.verdict === VERDICT.CONFLICTED) { + process.stderr.write( + `::error::This pull request cannot cleanly merge ${baseBranch}. Rebase onto current ${baseBranch} and push again; no CI will run until the conflict is resolved.\n`, + ); + const state = result.payload && typeof result.payload === 'object' ? result.payload.mergeable_state : undefined; + if (state) { + process.stderr.write(`::error::mergeable_state: ${state}\n`); + } + return 1; + } + + if (result.verdict === VERDICT.INDETERMINATE) { + process.stderr.write( + '::warning::GitHub did not report mergeability within the polling budget; proceeding (fail-open). scripts/ci-rebase-check.cjs remains the in-job backstop.\n', + ); + return 0; + } + + process.stdout.write(`PR mergeability preflight: ${result.verdict}\n`); + return 0; +} + +if (require.main === module) { + runMain(main); +} + +module.exports = { + VERDICT, + MAX_ATTEMPTS, + BASE_DELAY_MS, + classifyMergeability, + nextDelayMs, + resolveMergeability, + main, +}; diff --git a/scripts/ci-test-scope.cjs b/scripts/ci-test-scope.cjs index d9c9c935f..5602e2d15 100644 --- a/scripts/ci-test-scope.cjs +++ b/scripts/ci-test-scope.cjs @@ -39,6 +39,10 @@ const PROTECTED_WORKFLOWS = new Set([ 'mutation.yml', 'security-scan.yml', 'release.yml', + // #3833: the reusable gate every `pull_request` compute lane depends on — + // marking it inert would let a change to the gate itself ship without ever + // running the full matrix it is responsible for enforcing. + 'pr-mergeable-preflight.yml', ]); for (const wf of PROTECTED_WORKFLOWS) { if (INERT_WORKFLOWS.has(wf)) { @@ -341,19 +345,48 @@ function missingRuleTestFiles(rules) { return [...referenced].filter(f => !existsSync(path.join(__dirname, '..', f))).sort(); } -// Fail loudly at module load, mirroring the PROTECTED_WORKFLOWS check above — -// this fires on EVERY invocation of the CLI (including the real `changes` job -// in .github/workflows/test.yml), not only when a test suite happens to run. -{ - const missing = missingRuleTestFiles(RULES); - if (missing.length > 0) { - throw new Error( - `ci-test-scope: RULES reference test file(s) that do not exist on disk ` + - `(silent coverage hole — see #2758):\n ${missing.join('\n ')}`, - ); - } +/** + * Every PROTECTED_WORKFLOWS member that does not exist on disk. + * The name list and the real filenames are two surfaces over one fact + * (#3833): without this, renaming or deleting a gating workflow silently + * un-protects it and CI stays green while the protection is gone. Same + * shape and rationale as missingRuleTestFiles() above. + */ +function missingProtectedWorkflows(names = PROTECTED_WORKFLOWS) { + return [...names] + .filter(name => !existsSync(path.join(__dirname, '..', '.github', 'workflows', name))) + .sort(); } +/** + * Shared module-load assertion for the two "this list names something that + * does not exist on disk" guards above. Both are silent-coverage-hole guards: + * a name that no longer resolves stops selecting/protecting anything while CI + * stays green, so both must fail loudly at load rather than at test time. + */ +function assertNoneMissing(missing, summary, issueRef) { + if (missing.length === 0) return; + throw new Error(`ci-test-scope: ${summary} (${issueRef}):\n ${missing.join('\n ')}`); +} + +// Fail loudly at module load — this fires on EVERY invocation of the CLI +// (including the real `changes` job in .github/workflows/test.yml), not only +// when a test suite happens to run. +assertNoneMissing( + missingRuleTestFiles(RULES), + 'RULES reference test file(s) that do not exist on disk (silent coverage hole)', + 'see #2758', +); + +// A PROTECTED_WORKFLOWS entry naming a file that does not exist means the +// workflow was renamed or deleted without updating this list, silently +// un-protecting it while CI stays green. +assertNoneMissing( + missingProtectedWorkflows(PROTECTED_WORKFLOWS), + 'PROTECTED_WORKFLOWS reference workflow file(s) that do not exist on disk (silent protection hole)', + 'see #3833', +); + function usage() { return [ 'Usage:', @@ -590,4 +623,4 @@ if (require.main === module) { runMain(main); } -module.exports = { RULES, missingRuleTestFiles }; +module.exports = { RULES, missingRuleTestFiles, PROTECTED_WORKFLOWS, INERT_WORKFLOWS, missingProtectedWorkflows }; diff --git a/tests/ci-pr-mergeability.test.cjs b/tests/ci-pr-mergeability.test.cjs new file mode 100644 index 000000000..20ffa4733 --- /dev/null +++ b/tests/ci-pr-mergeability.test.cjs @@ -0,0 +1,955 @@ +'use strict'; + +// #3833 — PR mergeability preflight. +// +// Risk asymmetry drives this suite (see .gsd/phase/.../50-test-matrix.md): +// false positive (clean PR called CONFLICTED) => every compute lane in all +// eight gated workflows skips, on every PR. Repo-wide outage. +// false negative (conflict missed) => today's behavior; the per-job +// scripts/ci-rebase-check.cjs backstop still catches it. +// So the weight is on the false-positive surface: the single predicate +// `mergeable === false`, and every falsy-but-not-false value around it. + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const http = require('node:http'); +const yaml = require('js-yaml'); +const fc = require('fast-check'); + +const { createTempDir, cleanup } = require('./helpers.cjs'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); + +const ROOT = path.join(__dirname, '..'); +const SCRIPT = path.join(ROOT, 'scripts', 'ci-pr-mergeability.cjs'); +const WORKFLOWS_DIR = path.join(ROOT, '.github', 'workflows'); +const PREFLIGHT_WORKFLOW = 'pr-mergeable-preflight.yml'; +const PREFLIGHT_USES = `./.github/workflows/${PREFLIGHT_WORKFLOW}`; + +const { + VERDICT, + MAX_ATTEMPTS, + classifyMergeability, + nextDelayMs, + resolveMergeability, + main, +} = require('../scripts/ci-pr-mergeability.cjs'); + +// --------------------------------------------------------------------------- +// Fakes. The seams are parameters, not module patches — no global state, so +// every case is order-independent. +// --------------------------------------------------------------------------- + +/** A fetchPr fake that replays a script of responses; a value that is an Error is thrown. */ +function scriptedFetch(responses) { + const calls = []; + const fn = async (prNumber) => { + const index = calls.length; + calls.push(prNumber); + const next = index < responses.length ? responses[index] : responses[responses.length - 1]; + if (next instanceof Error) throw next; + return next; + }; + fn.calls = calls; + return fn; +} + +/** A sleep fake that records the delays it was asked for and never actually waits. */ +function recordingSleep() { + const delays = []; + const fn = async (ms) => { delays.push(ms); }; + fn.delays = delays; + return fn; +} + +const PR = { number: 4242 }; + +function resolveWith(overrides) { + return resolveMergeability({ + eventName: 'pull_request', + prNumber: PR.number, + ...overrides, + }); +} + +// --------------------------------------------------------------------------- +// A. classifyMergeability — pure +// --------------------------------------------------------------------------- + +describe('ci-pr-mergeability: classifyMergeability', () => { + test('classifies mergeable true as MERGEABLE', () => { + assert.equal(classifyMergeability({ mergeable: true }), VERDICT.MERGEABLE); + }); + + test('classifies mergeable false as CONFLICTED', () => { + assert.equal(classifyMergeability({ mergeable: false }), VERDICT.CONFLICTED); + }); + + test('classifies mergeable null as INDETERMINATE, never CONFLICTED', () => { + // GitHub's documented "background computation is in progress" sentinel. + assert.equal(classifyMergeability({ mergeable: null }), VERDICT.INDETERMINATE); + }); + + test('classifies mergeable undefined as INDETERMINATE', () => { + assert.equal(classifyMergeability({ mergeable: undefined }), VERDICT.INDETERMINATE); + }); + + test('classifies an absent mergeable key as INDETERMINATE', () => { + assert.equal(classifyMergeability({ id: 1 }), VERDICT.INDETERMINATE); + }); + + test('never treats a falsy non-false mergeable as CONFLICTED', () => { + // This is the row that forecloses `if (!mergeable) return CONFLICTED`. + for (const value of [0, -0, '', NaN, null, undefined]) { + assert.equal( + classifyMergeability({ mergeable: value }), + VERDICT.INDETERMINATE, + `falsy value ${String(value)} must not be a conflict`, + ); + } + }); + + test('never treats a truthy non-true mergeable as MERGEABLE', () => { + for (const value of [1, 'true', 'false', {}, [], 'yes']) { + assert.equal( + classifyMergeability({ mergeable: value }), + VERDICT.INDETERMINATE, + `truthy value ${JSON.stringify(value)} must not be a clean verdict`, + ); + } + }); + + test('mergeable_state does not override a null mergeable', () => { + assert.equal( + classifyMergeability({ mergeable: null, mergeable_state: 'dirty' }), + VERDICT.INDETERMINATE, + ); + }); + + test('blocked mergeable_state is not a conflict', () => { + // `blocked` means "required checks have not passed" — always true while + // this very job is running. Classifying on it self-deadlocks the pipeline. + assert.equal( + classifyMergeability({ mergeable: true, mergeable_state: 'blocked' }), + VERDICT.MERGEABLE, + ); + }); + + test('only mergeable decides the verdict, across every mergeable_state', () => { + for (const state of ['behind', 'unstable', 'draft', 'unknown', 'clean', 'dirty']) { + assert.equal( + classifyMergeability({ mergeable: true, mergeable_state: state }), + VERDICT.MERGEABLE, + `mergeable_state=${state} must not change a clean verdict`, + ); + } + }); + + test('degrades a non-object payload to INDETERMINATE without throwing', () => { + for (const value of [null, undefined, 0, 'str', true, 42]) { + assert.equal(classifyMergeability(value), VERDICT.INDETERMINATE); + } + }); + + test('VERDICT is frozen and its atom set is locked', () => { + assert.ok(Object.isFrozen(VERDICT)); + assert.deepEqual( + Object.keys(VERDICT).sort(), + ['CONFLICTED', 'INDETERMINATE', 'MERGEABLE', 'SKIPPED_NOT_A_PR'], + ); + }); + + test('CONFLICTED iff mergeable === false (property)', () => { + fc.assert( + fc.property(fc.anything(), (value) => { + const verdict = classifyMergeability({ mergeable: value }); + return (verdict === VERDICT.CONFLICTED) === (value === false); + }), + { seed: 3833, numRuns: 500, verbose: true }, + ); + }); +}); + +// --------------------------------------------------------------------------- +// B. nextDelayMs — pure +// --------------------------------------------------------------------------- + +describe('ci-pr-mergeability: nextDelayMs', () => { + test('first retry waits one base interval', () => { + assert.equal(nextDelayMs(1, { baseMs: 1000 }), 1000); + }); + + test('backoff is strictly increasing and finite across the budget', () => { + let previous = 0; + for (let attempt = 1; attempt < MAX_ATTEMPTS; attempt++) { + const delay = nextDelayMs(attempt, { baseMs: 1000 }); + assert.ok(Number.isFinite(delay), `delay for attempt ${attempt} must be finite`); + assert.ok(delay > previous, `delay must increase: ${delay} !> ${previous}`); + previous = delay; + } + }); + + test('clamps a non-positive attempt to one base interval', () => { + for (const attempt of [0, -1, -99, NaN, undefined]) { + assert.equal(nextDelayMs(attempt, { baseMs: 1000 }), 1000); + } + }); + + test('total backoff across the full budget stays within the job budget', () => { + let total = 0; + for (let attempt = 1; attempt < MAX_ATTEMPTS; attempt++) total += nextDelayMs(attempt); + // The preflight job is capped at 3 minutes; the sleeps alone must not + // approach it, or a slow API turns the optimization into a stall. + assert.ok(total < 60_000, `total backoff ${total}ms must stay under 60s`); + }); +}); + +// --------------------------------------------------------------------------- +// C. resolveMergeability — dependency-injected poll loop +// --------------------------------------------------------------------------- + +describe('ci-pr-mergeability: resolveMergeability', () => { + test('resolves on the first read without sleeping', async () => { + const fetchPr = scriptedFetch([{ mergeable: true }]); + const sleep = recordingSleep(); + const result = await resolveWith({ fetchPr, sleep }); + assert.equal(result.verdict, VERDICT.MERGEABLE); + assert.equal(fetchPr.calls.length, 1); + assert.equal(sleep.delays.length, 0); + }); + + test('polls past a cold null read', async () => { + const fetchPr = scriptedFetch([{ mergeable: null }, { mergeable: true }]); + const sleep = recordingSleep(); + const result = await resolveWith({ fetchPr, sleep }); + assert.equal(result.verdict, VERDICT.MERGEABLE); + assert.equal(fetchPr.calls.length, 2); + assert.equal(sleep.delays.length, 1); + }); + + test('stops polling the moment a conflict is known', async () => { + const fetchPr = scriptedFetch([{ mergeable: false }]); + const sleep = recordingSleep(); + const result = await resolveWith({ fetchPr, sleep }); + assert.equal(result.verdict, VERDICT.CONFLICTED); + assert.equal(fetchPr.calls.length, 1); + assert.equal(sleep.delays.length, 0); + }); + + test('a late false is still a conflict', async () => { + const fetchPr = scriptedFetch([ + { mergeable: null }, { mergeable: null }, { mergeable: false }, + ]); + const result = await resolveWith({ fetchPr, sleep: recordingSleep() }); + assert.equal(result.verdict, VERDICT.CONFLICTED); + assert.equal(fetchPr.calls.length, 3); + }); + + test('resolves at limit-1 attempts', async () => { + const responses = Array.from({ length: MAX_ATTEMPTS - 2 }, () => ({ mergeable: null })); + responses.push({ mergeable: true }); + const fetchPr = scriptedFetch(responses); + const result = await resolveWith({ fetchPr, sleep: recordingSleep() }); + assert.equal(result.verdict, VERDICT.MERGEABLE); + assert.equal(fetchPr.calls.length, MAX_ATTEMPTS - 1); + }); + + test('resolves on the final permitted attempt', async () => { + const responses = Array.from({ length: MAX_ATTEMPTS - 1 }, () => ({ mergeable: null })); + responses.push({ mergeable: true }); + const fetchPr = scriptedFetch(responses); + const result = await resolveWith({ fetchPr, sleep: recordingSleep() }); + assert.equal(result.verdict, VERDICT.MERGEABLE); + assert.equal(fetchPr.calls.length, MAX_ATTEMPTS); + }); + + test('stops at the budget and never makes a sixth call', async () => { + // The fake would answer null forever; the budget is what must stop it. + const fetchPr = scriptedFetch([{ mergeable: null }]); + const sleep = recordingSleep(); + const result = await resolveWith({ fetchPr, sleep }); + assert.equal(result.verdict, VERDICT.INDETERMINATE); + assert.equal(fetchPr.calls.length, MAX_ATTEMPTS); + assert.equal(sleep.delays.length, MAX_ATTEMPTS - 1); + }); + + test('honours a budget of one', async () => { + const fetchPr = scriptedFetch([{ mergeable: null }]); + const sleep = recordingSleep(); + const result = await resolveWith({ fetchPr, sleep, maxAttempts: 1 }); + assert.equal(result.verdict, VERDICT.INDETERMINATE); + assert.equal(fetchPr.calls.length, 1); + assert.equal(sleep.delays.length, 0); + }); + + test('a non-positive budget makes no API call', async () => { + for (const maxAttempts of [0, -1]) { + const fetchPr = scriptedFetch([{ mergeable: false }]); + const result = await resolveWith({ fetchPr, sleep: recordingSleep(), maxAttempts }); + assert.equal(result.verdict, VERDICT.INDETERMINATE); + assert.equal(fetchPr.calls.length, 0); + } + }); + + test('a throwing fetch degrades to INDETERMINATE', async () => { + const fetchPr = scriptedFetch([new Error('ECONNRESET')]); + const result = await resolveWith({ fetchPr, sleep: recordingSleep() }); + assert.equal(result.verdict, VERDICT.INDETERMINATE); + assert.equal(fetchPr.calls.length, MAX_ATTEMPTS); + }); + + test('recovers from a transient fetch error', async () => { + const fetchPr = scriptedFetch([new Error('503'), { mergeable: true }]); + const result = await resolveWith({ fetchPr, sleep: recordingSleep() }); + assert.equal(result.verdict, VERDICT.MERGEABLE); + assert.equal(fetchPr.calls.length, 2); + }); + + test('a conflict after a transient error is still reported', async () => { + const fetchPr = scriptedFetch([new Error('503'), { mergeable: false }]); + const result = await resolveWith({ fetchPr, sleep: recordingSleep() }); + assert.equal(result.verdict, VERDICT.CONFLICTED); + }); + + test('makes no API call on a push event', async () => { + const fetchPr = scriptedFetch([{ mergeable: false }]); + const sleep = recordingSleep(); + const result = await resolveMergeability({ + eventName: 'push', prNumber: PR.number, fetchPr, sleep, + }); + assert.equal(result.verdict, VERDICT.SKIPPED_NOT_A_PR); + assert.equal(fetchPr.calls.length, 0); + assert.equal(sleep.delays.length, 0); + }); + + test('makes no API call on workflow_dispatch', async () => { + const fetchPr = scriptedFetch([{ mergeable: false }]); + const result = await resolveMergeability({ + eventName: 'workflow_dispatch', prNumber: PR.number, fetchPr, sleep: recordingSleep(), + }); + assert.equal(result.verdict, VERDICT.SKIPPED_NOT_A_PR); + assert.equal(fetchPr.calls.length, 0); + }); + + test('an unknown event name makes no API call', async () => { + for (const eventName of ['', undefined, 'schedule', 'release']) { + const fetchPr = scriptedFetch([{ mergeable: false }]); + const result = await resolveMergeability({ + eventName, prNumber: PR.number, fetchPr, sleep: recordingSleep(), + }); + assert.equal(result.verdict, VERDICT.SKIPPED_NOT_A_PR); + assert.equal(fetchPr.calls.length, 0); + } + }); + + test('a missing PR number resolves INDETERMINATE without calling', async () => { + for (const prNumber of [undefined, null, 0, -1, NaN, '12']) { + const fetchPr = scriptedFetch([{ mergeable: true }]); + const result = await resolveMergeability({ + eventName: 'pull_request', prNumber, fetchPr, sleep: recordingSleep(), + }); + assert.equal(result.verdict, VERDICT.INDETERMINATE); + assert.equal(fetchPr.calls.length, 0); + } + }); + + test('successive resolutions do not share state', async () => { + const first = scriptedFetch([{ mergeable: null }]); + await resolveWith({ fetchPr: first, sleep: recordingSleep() }); + assert.equal(first.calls.length, MAX_ATTEMPTS); + + const second = scriptedFetch([{ mergeable: true }]); + const result = await resolveWith({ fetchPr: second, sleep: recordingSleep() }); + assert.equal(result.verdict, VERDICT.MERGEABLE); + assert.equal(second.calls.length, 1, 'the second run must start from a fresh budget'); + }); +}); + +// --------------------------------------------------------------------------- +// D. main() — integration through the process seam against a real local API. +// +// The CLI's real fetch path is exercised by pointing GITHUB_API_URL at a +// throwaway localhost server, so no production test-mode branch exists and +// nothing reaches api.github.com. +// --------------------------------------------------------------------------- + +const SENTINEL_TOKEN = 'ghs_sentinel_must_never_be_echoed_3833'; + +/** Start a one-shot API stub. `handler(requestCount)` returns { status, body }. */ +async function startApiStub(handler) { + let requestCount = 0; + const server = http.createServer((req, res) => { + const { status, body } = handler(requestCount++); + res.writeHead(status, { 'content-type': 'application/json' }); + res.end(typeof body === 'string' ? body : JSON.stringify(body)); + }); + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); + const { port } = server.address(); + return { + url: `http://127.0.0.1:${port}`, + close: () => new Promise((resolve) => server.close(resolve)), + get requestCount() { return requestCount; }, + }; +} + +function runCli(env, { outputPath } = {}) { + return runNode([SCRIPT], { + cwd: ROOT, + timeoutMs: PROBE_TIMEOUT_MS, + env: { + ...process.env, + GITHUB_REPOSITORY: 'open-gsd/gsd-core', + GITHUB_TOKEN: SENTINEL_TOKEN, + GITHUB_BASE_REF: 'next', + PR_NUMBER: String(PR.number), + GSD_PR_MERGEABILITY_BASE_DELAY_MS: '1', + ...(outputPath ? { GITHUB_OUTPUT: outputPath } : {}), + ...env, + }, + }); +} + +// `runNode` (tests/helpers/process-seam.cjs) is spawnSync: it blocks this +// process's event loop for the whole child lifetime. `startApiStub` is an +// http.createServer living in THIS process, so while spawnSync blocks, the +// stub can never accept the child's connection — the child's fetch hangs on +// every attempt until its own AbortSignal.timeout fires, five attempts in a +// row, blowing past PROBE_TIMEOUT_MS. The observed symptom was exitCode:null +// with empty stdout/stderr (caught by the remote runner, not locally, since +// the deadlock's exact timing depends on scheduler contention). Any test that +// needs the stub must instead call main() in-process, which keeps this +// process's event loop live so the stub can actually answer. +/** + * Invoke main() in-process with a controlled environment, capturing writes. + * See the comment above for why this exists instead of routing through the + * runNode/spawnSync seam whenever an in-process HTTP stub is involved. + */ +async function callMain(t, env, { outputPath } = {}) { + const overrides = { + GITHUB_REPOSITORY: 'open-gsd/gsd-core', + GITHUB_TOKEN: SENTINEL_TOKEN, + GITHUB_BASE_REF: 'next', + PR_NUMBER: String(PR.number), + GSD_PR_MERGEABILITY_BASE_DELAY_MS: '1', + ...(outputPath ? { GITHUB_OUTPUT: outputPath } : {}), + ...env, + }; + const saved = new Map(); + for (const key of Object.keys(overrides)) saved.set(key, process.env[key]); + Object.assign(process.env, overrides); + t.after(() => { + for (const [key, value] of saved) { + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + }); + + const stdout = []; + const stderr = []; + t.mock.method(process.stdout, 'write', (chunk) => { stdout.push(String(chunk)); return true; }); + t.mock.method(process.stderr, 'write', (chunk) => { stderr.push(String(chunk)); return true; }); + + const code = await main([]); + return { code, stdout: stdout.join(''), stderr: stderr.join('') }; +} + +function readOutputs(outputPath) { + const raw = fs.readFileSync(outputPath, 'utf8'); + const outputs = {}; + for (const line of raw.split(/\r?\n/)) { + const index = line.indexOf('='); + if (index > 0) outputs[line.slice(0, index)] = line.slice(index + 1); + } + return outputs; +} + +// Split by what each case actually needs: stub-backed cases call main() +// in-process via callMain() (spawnSync would deadlock against the +// in-process HTTP stub — see the comment above callMain()); the remaining +// argv/exit-code-as-a-process cases stay on the runCli()/runNode seam since +// they assert real process argv parsing and exit-code plumbing. +describe('ci-pr-mergeability: CLI', () => { + test('exits 0 and writes a skip verdict on a push event', (t) => { + const dir = createTempDir('mergeability-skip-'); + t.after(() => cleanup(dir)); + const outputPath = path.join(dir, 'gh-output'); + + const result = runCli({ GITHUB_EVENT_NAME: 'push' }, { outputPath }); + + assert.equal(result.exitCode, 0, result.stderr); + const outputs = readOutputs(outputPath); + assert.equal(outputs.verdict, VERDICT.SKIPPED_NOT_A_PR); + assert.equal(outputs.mergeable, 'true'); + }); + + test('exits 1 and annotates the conflict', async (t) => { + const dir = createTempDir('mergeability-conflict-'); + const api = await startApiStub(() => ({ status: 200, body: { mergeable: false, mergeable_state: 'dirty' } })); + t.after(async () => { await api.close(); cleanup(dir); }); + const outputPath = path.join(dir, 'gh-output'); + + const result = await callMain( + t, + { GITHUB_EVENT_NAME: 'pull_request', GITHUB_API_URL: api.url }, + { outputPath }, + ); + + assert.equal(result.code, 1, `stdout: ${result.stdout}\nstderr: ${result.stderr}`); + const combined = `${result.stdout}${result.stderr}`; + assert.ok(combined.includes('::error::'), 'must emit a workflow error annotation'); + assert.ok(combined.includes('next'), 'the annotation must name the base branch'); + assert.equal(readOutputs(outputPath).verdict, VERDICT.CONFLICTED); + }); + + test('exits 0 on a mergeable PR', async (t) => { + const dir = createTempDir('mergeability-clean-'); + const api = await startApiStub(() => ({ status: 200, body: { mergeable: true, mergeable_state: 'clean' } })); + t.after(async () => { await api.close(); cleanup(dir); }); + const outputPath = path.join(dir, 'gh-output'); + + const result = await callMain( + t, + { GITHUB_EVENT_NAME: 'pull_request', GITHUB_API_URL: api.url }, + { outputPath }, + ); + + assert.equal(result.code, 0, result.stderr); + const outputs = readOutputs(outputPath); + assert.equal(outputs.verdict, VERDICT.MERGEABLE); + assert.equal(outputs.mergeable, 'true'); + }); + + test('fails open with a warning when mergeability is unknown', async (t) => { + const dir = createTempDir('mergeability-unknown-'); + const api = await startApiStub(() => ({ status: 200, body: { mergeable: null } })); + t.after(async () => { await api.close(); cleanup(dir); }); + const outputPath = path.join(dir, 'gh-output'); + + const result = await callMain( + t, + { GITHUB_EVENT_NAME: 'pull_request', GITHUB_API_URL: api.url }, + { outputPath }, + ); + + assert.equal(result.code, 0, `an unknown mergeability must not block: ${result.stderr}`); + const combined = `${result.stdout}${result.stderr}`; + assert.ok(combined.includes('::warning::'), 'must warn'); + assert.ok(!combined.includes('::error::'), 'must not emit an error annotation'); + assert.equal(readOutputs(outputPath).verdict, VERDICT.INDETERMINATE); + assert.equal(api.requestCount, MAX_ATTEMPTS, 'must exhaust exactly the budget'); + }); + + test('fails open when the API rejects the read', async (t) => { + const dir = createTempDir('mergeability-403-'); + const api = await startApiStub(() => ({ status: 403, body: { message: 'Resource not accessible' } })); + t.after(async () => { await api.close(); cleanup(dir); }); + const outputPath = path.join(dir, 'gh-output'); + + const result = await callMain( + t, + { GITHUB_EVENT_NAME: 'pull_request', GITHUB_API_URL: api.url }, + { outputPath }, + ); + + assert.equal(result.code, 0, 'an unreadable PR is not an unmergeable PR'); + assert.equal(readOutputs(outputPath).verdict, VERDICT.INDETERMINATE); + }); + + test('fails open when the API body is not a JSON object', async (t) => { + const dir = createTempDir('mergeability-badjson-'); + const api = await startApiStub(() => ({ status: 200, body: 'not json at all' })); + t.after(async () => { await api.close(); cleanup(dir); }); + const outputPath = path.join(dir, 'gh-output'); + + const result = await callMain( + t, + { GITHUB_EVENT_NAME: 'pull_request', GITHUB_API_URL: api.url }, + { outputPath }, + ); + + assert.equal(result.code, 0); + assert.equal(readOutputs(outputPath).verdict, VERDICT.INDETERMINATE); + }); + + test('works when GITHUB_OUTPUT is not set', (t) => { + const dir = createTempDir('mergeability-nooutput-'); + t.after(() => cleanup(dir)); + + const result = runCli({ GITHUB_EVENT_NAME: 'push', GITHUB_OUTPUT: '' }); + + assert.equal(result.exitCode, 0, result.stderr); + }); + + test('an unwritable GITHUB_OUTPUT does not change the exit code', async (t) => { + // A failure while REPORTING the verdict must never invert the gate. + // Injected by pointing at a path whose parent does not exist — no mode-bit + // tricks, which root Docker/CI silently bypasses. + const dir = createTempDir('mergeability-badout-'); + const api = await startApiStub(() => ({ status: 200, body: { mergeable: false } })); + t.after(async () => { await api.close(); cleanup(dir); }); + const outputPath = path.join(dir, 'no-such-dir', 'gh-output'); + + const result = await callMain( + t, + { GITHUB_EVENT_NAME: 'pull_request', GITHUB_API_URL: api.url }, + { outputPath }, + ); + + assert.equal(result.code, 1, 'a conflict must still exit 1 when the output write fails'); + }); + + test('never echoes the token', async (t) => { + const dir = createTempDir('mergeability-token-'); + const api = await startApiStub(() => ({ status: 500, body: { message: 'boom' } })); + t.after(async () => { await api.close(); cleanup(dir); }); + + const result = await callMain( + t, + { GITHUB_EVENT_NAME: 'pull_request', GITHUB_API_URL: api.url }, + { outputPath: path.join(dir, 'gh-output') }, + ); + + assert.ok(!`${result.stdout}${result.stderr}`.includes(SENTINEL_TOKEN)); + }); + + test('reports a failure without a stack trace', async (t) => { + const dir = createTempDir('mergeability-nostack-'); + const api = await startApiStub(() => ({ status: 200, body: { mergeable: false } })); + t.after(async () => { await api.close(); cleanup(dir); }); + + const result = await callMain( + t, + { GITHUB_EVENT_NAME: 'pull_request', GITHUB_API_URL: api.url }, + { outputPath: path.join(dir, 'gh-output') }, + ); + + assert.ok(!`${result.stdout}${result.stderr}`.includes(' at '), 'no raw stack frames'); + }); + + test('prints usage', () => { + const result = runNode([SCRIPT, '--help'], { cwd: ROOT, timeoutMs: PROBE_TIMEOUT_MS }); + assert.equal(result.exitCode, 0); + assert.ok(/Usage/i.test(result.stdout)); + }); + + test('rejects an unknown argument', () => { + const result = runNode([SCRIPT, '--nope'], { cwd: ROOT, timeoutMs: PROBE_TIMEOUT_MS }); + assert.notEqual(result.exitCode, 0); + assert.ok(`${result.stdout}${result.stderr}`.includes('--nope')); + }); +}); + +// --------------------------------------------------------------------------- +// E. Workflow wiring — structural assertions on the parsed YAML object graph. +// +// CONTRIBUTING.md:842 bans raw-text matching on test outputs regardless of +// whether the text came from a source file — broader than local/no-source-grep's +// .cjs/.js/.ts scope. Every assertion below reads the parsed object graph +// (yaml.load), not the raw file text, EXCEPT the single absence check further +// down ('security and policy lanes are never gated on mergeability'): a +// structural read cannot see a comment or an unreachable key, so that one +// test deliberately keeps a raw scan, justified inline at its call site. +// --------------------------------------------------------------------------- + +/** workflow file -> job ids that must be gated on the preflight. */ +const GATED = Object.freeze({ + 'test.yml': ['lint-tests', 'test', 'test-inert', 'test-full', 'coverage-gate', 'qa-loop-walk', 'required-tests'], + 'install-smoke.yml': ['smoke', 'smoke-unpacked'], + 'mutation.yml': ['detect', 'mutation-gate'], + 'security-scan.yml': ['security'], + 'docs-required.yml': ['docs-lint'], + 'changeset-required.yml': ['changeset-lint'], + 'default-flip-documentation.yml': ['default-flip-documentation'], + 'branch-naming.yml': ['check-branch'], +}); + +/** Lanes that must keep running on a conflicted PR — security and policy. */ +const NEVER_GATED = Object.freeze([ + 'auto-close-unsolicited-prs.yml', + 'close-draft-prs.yml', + 'pr-target-validator.yml', + 'pr-title-validator.yml', + 'pr-template-format.yml', + 'require-issue-link.yml', + 'dismiss-unauthorized-pr-approvals.yml', +]); + +function loadWorkflow(name) { + return yaml.load(fs.readFileSync(path.join(WORKFLOWS_DIR, name), 'utf8')); +} + +function rawWorkflow(name) { + return fs.readFileSync(path.join(WORKFLOWS_DIR, name), 'utf8'); +} + +function needsOf(job) { + const needs = job && job.needs; + if (!needs) return []; + return Array.isArray(needs) ? needs : [needs]; +} + +function preflightJobIds(doc) { + return Object.entries(doc.jobs || {}) + .filter(([, job]) => job && job.uses === PREFLIGHT_USES) + .map(([id]) => id); +} + +describe('ci-pr-mergeability: workflow wiring', () => { + test('the preflight workflow exists and is call-only', () => { + const doc = loadWorkflow(PREFLIGHT_WORKFLOW); + // `on: workflow_call` parses to the key `true` under YAML 1.1 unless quoted; + // accept either shape rather than pinning the parser's quirk. + const triggers = doc.on || doc[true]; + assert.deepEqual(Object.keys(triggers), ['workflow_call']); + assert.equal(doc.concurrency, undefined, 'a matching concurrency group would cancel the caller'); + }); + + function findCheckoutStep() { + const doc = loadWorkflow(PREFLIGHT_WORKFLOW); + const job = doc.jobs.mergeability; + const step = (job.steps || []).find( + (s) => typeof s.uses === 'string' && s.uses.startsWith('actions/checkout@'), + ); + assert.ok(step, 'the mergeability job must have an actions/checkout step'); + return step; + } + + test('the preflight checks out the base sha, never the merge ref', () => { + const step = findCheckoutStep(); + const ref = step.with && step.with.ref; + assert.equal(typeof ref, 'string'); + // For a CONFLICTED PR refs/pull/N/merge is stale or absent, so a default + // checkout fails on exactly the PRs this job exists to diagnose; pinning + // the base sha also means no PR-supplied code is fetched or executed. + assert.ok( + ref.includes('github.event.pull_request.base.sha'), + 'the checkout must pin ref: to pull_request.base.sha', + ); + }); + + test('the preflight checkout is shallow, sparse, and credential-free', () => { + const step = findCheckoutStep(); + const { with: withArgs } = step; + assert.equal(withArgs['fetch-depth'], 1, 'fetch-depth must be the number 1'); + // The point is it checks out only what it runs. + assert.equal(typeof withArgs['sparse-checkout'], 'string'); + assert.equal(withArgs['sparse-checkout'].trim(), 'scripts'); + assert.equal(withArgs['persist-credentials'], false, 'must not persist credentials'); + }); + + test('the preflight fails open when the script is absent at the base sha', () => { + // The checkout above pins ref: to pull_request.base.sha, so `node + // scripts/ci-pr-mergeability.cjs` runs the copy that exists on the BASE + // branch, not on the PR. On the very PR that introduces this script, the + // base has no such file; the same is true for any PR whose base predates + // it. An absent script is not evidence of a conflict — it is undetermined + // — so the run step must take the same fail-open path as an unknown + // `mergeable`, not fail the job. That arm only ever fires while the base + // lacks the script, so once the fix lands on the base it goes silent and + // looks, to a future reader, like dead code safe to delete. This test + // exists to keep it from being deleted. + const doc = loadWorkflow(PREFLIGHT_WORKFLOW); + const step = (doc.jobs.mergeability.steps || []).find((s) => s.id === 'check'); + assert.ok(step, 'the mergeability job must have a step with id: check'); + const { run } = step; + assert.equal(typeof run, 'string', 'the check step must have a run: script'); + + assert.match( + run, + /if\s+\[\s+!\s+-f\s+scripts\/ci-pr-mergeability\.cjs\s+\]/, + 'the script must guard on the absence of scripts/ci-pr-mergeability.cjs before invoking it', + ); + assert.ok( + run.includes('::warning::'), + 'an absent script at the base sha must warn, not error', + ); + assert.ok( + !run.includes('::error::'), + 'an absent script at the base sha must not emit an error annotation', + ); + assert.ok( + run.includes('verdict=INDETERMINATE'), + 'the bootstrap arm must report verdict=INDETERMINATE, the same as any other unresolved read', + ); + assert.ok( + run.includes('mergeable=true'), + 'the bootstrap arm must fail open with mergeable=true so downstream gates are not blocked', + ); + assert.ok( + run.includes('exit 0'), + 'the bootstrap arm must exit 0 so the job itself does not go red on a missing script', + ); + assert.ok( + run.includes('node scripts/ci-pr-mergeability.cjs'), + 'the normal path must still invoke the real script once it exists at the base sha', + ); + assert.ok( + !run.includes('${{'), + 'CONTRIBUTING.md: run: blocks must not interpolate a GitHub expression', + ); + }); + + for (const [name, jobIds] of Object.entries(GATED)) { + test(`${name} calls the preflight`, () => { + const ids = preflightJobIds(loadWorkflow(name)); + assert.equal(ids.length, 1, `${name} must have exactly one preflight caller job, found ${ids.length}`); + }); + + test(`${name}: every gated compute job needs the preflight`, () => { + const doc = loadWorkflow(name); + const [callerId] = preflightJobIds(doc); + for (const jobId of jobIds) { + const job = (doc.jobs || {})[jobId]; + assert.ok(job, `${name}: job "${jobId}" not found — the gate list has drifted from the workflow`); + assert.ok( + needsOf(job).includes(callerId), + `${name}: job "${jobId}" must declare needs: ${callerId}`, + ); + } + }); + + test(`${name}: the preflight caller grants the scopes the preflight needs`, () => { + // A calling job's `permissions` REPLACES the workflow default; it does + // not merge. branch-naming.yml's workflow default is `{}`, so an implicit + // inherit would starve the API read. + const doc = loadWorkflow(name); + const [callerId] = preflightJobIds(doc); + const permissions = (doc.jobs || {})[callerId].permissions; + assert.equal(permissions && permissions['contents'], 'read', `${name}: caller needs contents: read`); + assert.equal(permissions && permissions['pull-requests'], 'read', `${name}: caller needs pull-requests: read`); + }); + } + + test('release.yml grants install-smoke the scopes its nested preflight requests', () => { + // release.yml -> install-smoke.yml -> pr-mergeable-preflight.yml is a + // nested reusable-workflow chain. A nested job may only DOWNGRADE the + // caller's permissions, so a request the caller never granted fails the + // whole release run before a single step executes. + const doc = loadWorkflow('release.yml'); + const callers = Object.entries(doc.jobs || {}) + .filter(([, job]) => job && job.uses === './.github/workflows/install-smoke.yml'); + assert.ok(callers.length >= 2, 'expected release.yml to call install-smoke.yml'); + for (const [id, job] of callers) { + assert.equal(job.permissions && job.permissions['pull-requests'], 'read', + `release.yml job "${id}" must grant pull-requests: read for the nested preflight`); + } + }); + + test('security and policy lanes are never gated on mergeability', () => { + // Deliberate raw scan, not a structural read: this asserts ABSENCE + // anywhere in the file, and the parsed object graph cannot see a + // commented-out or otherwise unreachable reference — exactly what this + // test must still catch. + for (const name of NEVER_GATED) { + const raw = rawWorkflow(name); + assert.ok( + !raw.includes(PREFLIGHT_WORKFLOW), + `${name} must keep running on a conflicted PR — a gated auto-close lets a drive-by PR evade it`, + ); + } + }); + + test('no run block added by this change interpolates a GitHub expression', () => { + // CONTRIBUTING.md, final line: no ${{ }} in `run:` blocks — bind via env:. + for (const name of [PREFLIGHT_WORKFLOW, 'mutation.yml']) { + const doc = loadWorkflow(name); + for (const [jobId, job] of Object.entries(doc.jobs || {})) { + for (const step of (job.steps || [])) { + if (typeof step.run !== 'string') continue; + assert.ok( + !step.run.includes('${{'), + `${name}: jobs.${jobId} has a run block interpolating an expression; bind it via env:`, + ); + } + } + } + }); + + test('required-tests reports red rather than absent when the preflight fails', () => { + const doc = loadWorkflow('test.yml'); + const job = doc.jobs['required-tests']; + assert.equal(job.if, 'always()', 'required-tests must still evaluate when upstream jobs skip'); + const script = (job.steps || []).map((s) => s.run || '').join('\n'); + assert.ok( + /PREFLIGHT_RESULT/.test(script), + 'required-tests must have an explicit preflight arm so the required context goes red, not absent', + ); + }); + + test('a gated job whose if() overrides the implicit success() must check the preflight result itself', () => { + // GitHub Actions applies an implicit success() to a job-level `if:` unless + // that `if:` already contains a status-check function: "By default, GitHub + // Actions applies a success() check to steps unless another status function + // is specified" and "you must explicitly include failure() to override the + // default success() check that is otherwise applied automatically." + // + // So for a job with a plain `if:` (no status function), `needs: preflight` + // alone correctly skips it when the preflight fails — nothing extra is + // required. But the moment a job's `if:` contains always(), failure(), or + // cancelled(), that implicit success() is gone and `needs:` stops gating + // it. Such a job runs even on a conflicted PR and must therefore check the + // preflight result itself. + // + // Today exactly two gated jobs are in that category and both already + // handle it: test.yml's required-tests (if: always(), with a + // PREFLIGHT_RESULT arm) and mutation.yml's mutation-gate (if: always() && + // ..., with a PREFLIGHT arm). This test exists to catch a FUTURE edit that + // adds always() to, say, `test` or `coverage-gate` — which would silently + // make the whole gate inert on the most expensive lanes while every + // existing assertion kept passing. + const STATUS_FUNCTION = /always\(\)|failure\(\)|cancelled\(\)/; + const violations = []; + let inspected = 0; + for (const [name, jobIds] of Object.entries(GATED)) { + const doc = loadWorkflow(name); + for (const jobId of jobIds) { + const job = (doc.jobs || {})[jobId]; + const condition = typeof job.if === 'string' ? job.if : ''; + if (!STATUS_FUNCTION.test(condition)) continue; + inspected++; + const checksPreflightInIf = /needs\.preflight\.result/.test(condition); + const stepsText = JSON.stringify(job.steps || []); + const checksPreflightInSteps = /PREFLIGHT/.test(stepsText); + if (!checksPreflightInIf && !checksPreflightInSteps) { + violations.push(`${name}:${jobId} (if: ${condition})`); + } + } + } + assert.ok( + inspected > 0, + 'expected at least one gated job with a status-function if(); a vacuous pass here means the invariant is no longer being checked', + ); + assert.deepEqual( + violations, + [], + `these jobs override the implicit success() with a status function in if:, so \`needs: preflight\` no longer gates them — each must test needs.preflight.result (or read PREFLIGHT_RESULT in a step) itself: ${violations.join(', ')}`, + ); + }); +}); + +// --------------------------------------------------------------------------- +// F. ci-test-scope ripple — the preflight workflow must never be marked inert. +// --------------------------------------------------------------------------- + +describe('ci-pr-mergeability: ci-test-scope protection', () => { + const scope = require('../scripts/ci-test-scope.cjs'); + + test('the preflight workflow is protected from being marked inert', () => { + assert.ok( + scope.PROTECTED_WORKFLOWS.has(PREFLIGHT_WORKFLOW), + 'a workflow that gates every compute lane must never be demoted to the inert lane', + ); + }); + + test('every protected workflow name refers to a real file', () => { + // Parity assertion: the literal name list and the real filenames are two + // surfaces over one fact. Without this, a rename silently un-protects a + // gating workflow and CI stays green. + const missing = [...scope.PROTECTED_WORKFLOWS] + .filter((name) => !fs.existsSync(path.join(WORKFLOWS_DIR, name))) + .sort(); + assert.deepEqual(missing, [], `PROTECTED_WORKFLOWS names files that do not exist: ${missing.join(', ')}`); + }); +});