From 5f38cddf04d1dda8e45f0cffa7c1be2b448b8b79 Mon Sep 17 00:00:00 2001 From: Bart Van Bos Date: Sat, 14 Nov 2026 01:20:15 +0100 Subject: [PATCH] feat: pr-changes.yml and the required-gate action for PR-only CI A repository's PR workflow (pr.yml) runs its heavy validation once per pull request revision instead of on the branch push, the pull request and the merge to main. Two shared pieces: - pr-changes.yml classifies the files the pull request's merge commit changes against its first parent: documentation only, or full. Unknown means full: deletions, renames, symlinks, agent instructions, fixtures, dot directories, code-paths, non-PR events and unverifiable checkouts. - actions/required-gate is the single required check, "OT Fabric Required Checks". It fails on any failed, cancelled, unexpectedly skipped, missing or malformed result, on an undeclared job, and without a classification. Selftest: the extracted classifier on 30 synthetic pull requests, the gate on 28 cases and all 768 result combinations, and both on real GitHub job results. Existing workflows are unchanged. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/README.md | 134 ++++++++++++++++++++- .github/workflows/pr-changes.yml | 188 ++++++++++++++++++++++++++++++ .github/workflows/selftest.yml | 123 +++++++++++++++++++ AGENTS.md | 9 +- README.md | 4 +- actions/README.md | 36 ++++++ actions/required-gate/action.yml | 37 ++++++ actions/required-gate/gate.py | 142 ++++++++++++++++++++++ scripts/selftest/pr-changes.sh | 116 ++++++++++++++++++ scripts/selftest/required-gate.sh | 96 +++++++++++++++ 10 files changed, 878 insertions(+), 7 deletions(-) create mode 100644 .github/workflows/pr-changes.yml create mode 100644 actions/required-gate/action.yml create mode 100755 actions/required-gate/gate.py create mode 100755 scripts/selftest/pr-changes.sh create mode 100755 scripts/selftest/required-gate.sh diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 903f034..00e85e8 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -4,7 +4,7 @@ Shared [reusable workflows](https://docs.github.com/en/actions/sharing-automatio The `OTFABRIC_READ_TOKEN` secret gives access to private `github.com/otfabric/*` modules. In `go-ci.yml` it is needed when a module has private dependencies, and by the policy job when `.otfabric/policy.yaml` exists. The token must be able to read `otfabric/engineering`. -**Permissions.** The organisation's default `GITHUB_TOKEN` is read-only, and a caller must grant at least what a called workflow requests, or the run fails at startup with no job started. Grant `contents: read` and `id-token: write` for `go-ci.yml` (Codecov OIDC), `contents: read` for `docs-check.yml`, and `contents: write` for the release workflows. +**Permissions.** The organisation's default `GITHUB_TOKEN` is read-only, and a caller must grant at least what a called workflow requests, or the run fails at startup with no job started. Grant `contents: read` and `id-token: write` for `go-ci.yml` (Codecov OIDC), `contents: read` for `docs-check.yml` and `pr-changes.yml`, and `contents: write` for the release workflows. **Versions.** `v3` turns on govulncheck, tests `cmd/`, uses atomic coverage and starts the matrix at the go directive. `v2` and `v2.1.0` were moved to the same commit by the maintainer on 2026-10-30, so they have these defaults too. @@ -20,6 +20,9 @@ Reusable workflows share whole pipelines (the caller's job is `uses:` one of the | [`go-package-release.yml`](go-package-release.yml) | Semver-tagged release for Go libraries / packages | | [`go-binary-release.yml`](go-binary-release.yml) | Cross-compiled binary release with SHA-256 checksums | | [`docs-check.yml`](docs-check.yml) | [docsync](https://github.com/otfabric/docsync) and otfabric-policy, on every push, Markdown-only ones included | +| [`pr-changes.yml`](pr-changes.yml) | Classifies a pull request: documentation only, or full validation; for the PR workflow below | + +The required check of a PR workflow is the composite action [`actions/required-gate`](../../actions/README.md#required-check-of-a-pr-workflow). ## Which workflows to use @@ -153,7 +156,134 @@ Both release workflows support an optional `RELEASE.md` file in the repo root. W Sections are delimited by `---`. Only the section matching the tag being released is extracted. -## Markdown-only changes +## PR workflow and the required check + +A repository that adopts it runs its heavy validation once per pull request +revision, and not again on the push of the branch or on the merge to `main`: + +| Event | What runs | +|---|---| +| Push to a feature branch | Nothing | +| Pull request opened, synchronized, reopened | `pr.yml`: Docs and Policy always; CI, interop and the other heavy jobs unless the pull request is documentation only; then `OT Fabric Required Checks` | +| Push to `main` (the merge) | `docs.yml` only (docsync and policy, about a minute) | +| Schedule, `workflow_dispatch` | Unchanged: nightly, self-interop; `ci.yml` and `interop.yml` can also be dispatched on any branch | +| Release | Unchanged: the release workflows test the release commit in full | + +One workflow, `pr.yml`, runs on every pull request with no path filter, and +its last job is the only required check. The heavy workflows keep their jobs +and become reusable (`on: workflow_call`), so nothing is copied: + +```yaml +# .github/workflows/pr.yml +name: PR +on: + pull_request: + +permissions: + contents: read + +# A new revision cancels the run of the previous one; other workflows +# (release, nightly, dispatch) are in other groups. +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + changes: + uses: otfabric/.github/.github/workflows/pr-changes.yml@v3.7.0 + + docs: + uses: ./.github/workflows/docs.yml + secrets: inherit + + ci: + needs: changes + if: needs.changes.outputs.full == 'true' + permissions: + contents: read + id-token: write + uses: ./.github/workflows/ci.yml + secrets: inherit + + required: + name: OT Fabric Required Checks + needs: [changes, docs, ci] + if: always() + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: otfabric/.github/actions/required-gate@v3.7.0 + with: + needs: ${{ toJSON(needs) }} + required: changes docs + required-for-code: ci +``` + +The rules the shape depends on: + +- **No path filter on `pr.yml`.** A required check whose workflow a filter + skipped never reports, and the pull request waits for it forever. +- **The gate has `if: always()`.** It runs when a job failed or the run was + cancelled, and fails then. A skipped job counts as passed for a required + check, so the gate must never be skipped; `!cancelled()` would skip it. +- **The gate needs every other job**, and lists each in `required` (must + succeed on every pull request) or `required-for-code` (must succeed unless + the pull request is documentation only, and then must be skipped). A job + in `needs` that is in neither list, or a listed job missing from `needs`, + fails the gate, so a job cannot be added without an expectation. +- **The gate job is defined in `pr.yml` itself**, so the check is named + `OT Fabric Required Checks` with no prefix. A gate inside a called + workflow would report as ` / `. +- **The heavy jobs depend on `changes`** and run when `full` is `"true"`. If + `changes` fails they are skipped and the gate fails: rerun the failed jobs. +- **Keep the triggers** to the defaults (`opened`, `synchronize`, + `reopened`). `edited` would rerun everything on a title change. A pull + request whose base is changed to `main` gets a run on its next push, or + close and reopen it. + +`pr-changes.yml` classifies the files the pull request's merge commit +changes, compared with its first parent: exactly the tree the jobs test. +Documentation only means every changed file is an added or modified regular +`*.md` file, not in a dot directory, `testdata/` or `vendor/`, not +`AGENTS.md`, `CLAUDE.md` or `GEMINI.md`, and not matched by `code-paths`. +A deletion, a rename, a symlink, any other file, a run that is not a pull +request, or a checkout it cannot verify gives `full: "true"`. List Markdown +that a test or generator reads in `code-paths` (go-iec61850's +`readme_test.go` reads `README.md`): + +```yaml + changes: + uses: otfabric/.github/.github/workflows/pr-changes.yml@v3.7.0 + with: + code-paths: | + README.md + docs/generated/*.md +``` + +Patterns are `fnmatch` patterns on the whole path: `*` also matches `/`, so +`*.md` matches every Markdown file and `README.md` only the root one. + +### Migrating a repository + +1. Make each heavy workflow reusable: replace its `push` and `pull_request` + triggers with `workflow_call` and `workflow_dispatch`. Its jobs, inputs + and pins stay as they are. +2. `docs.yml`: keep `push: branches: [main]`, replace `pull_request` with + `workflow_call`. +3. Add `pr.yml` as above, with one job per reusable workflow and one entry + for each in the gate. Grant each job the permissions its workflow + requests (`id-token: write` for go-ci, `packages: read` for GHCR pulls). +4. Open the pull request. It runs `pr.yml` from the branch: check that + `OT Fabric Required Checks` reports, and that its summary lists every job. +5. Only then require the check, by the exact name, in a ruleset (strict: + the branch must be up to date with `main`). Never require a job of a + called workflow or of a workflow with a path filter. + +Rollback: restore the previous triggers of the heavy workflows and delete +`pr.yml`; first remove the required check from the ruleset, or every open +pull request waits for it. + +## Markdown-only changes (repositories without a PR workflow) A push or pull request that changes only Markdown needs no build, tests or interop run. The heavy workflows of a repository skip it: diff --git a/.github/workflows/pr-changes.yml b/.github/workflows/pr-changes.yml new file mode 100644 index 0000000..b636866 --- /dev/null +++ b/.github/workflows/pr-changes.yml @@ -0,0 +1,188 @@ +# yaml-language-server: $schema=https://json.schemastore.org/github-workflow.json +# +# PR Changes — Reusable Workflow +# +# Classifies a pull request for the PR orchestration workflow of a repository: +# documentation only, or everything else. The heavy jobs of the caller (Go CI, +# interop, generated code) run when `full` is "true"; the documentation and +# policy jobs run on every pull request. +# +# Unknown means full. A file counts as documentation only when it is an added +# or modified regular Markdown file outside dot directories, testdata/ and +# vendor/, is not agent instructions (AGENTS.md, CLAUDE.md), and matches no +# pattern in code-paths. Anything else, a deletion or a rename, a run that is +# not a pull request, and a checkout that is not the pull request's merge +# commit give full: "true". The step never fails on a classification problem; +# it reports the reason instead. +# +# The changed files are those of the merge commit GitHub tests (its first +# parent is the base branch), so they are exactly what the heavy jobs check +# out: no API pagination, no file limit, no token scope beyond contents: read. +# +# Jobs: +# classify — "Classify changes": outputs full and reason, a table in the +# job summary. +# +# Caller example (in the repository's PR workflow): +# +# jobs: +# changes: +# uses: otfabric/.github/.github/workflows/pr-changes.yml@v3 +# with: +# # Markdown that a test or generator reads counts as code. +# code-paths: | +# README.md + +name: PR Changes + +on: + workflow_call: + inputs: + code-paths: + description: >- + Newline-separated fnmatch patterns, matched against the whole path + (`*` also matches `/`). A Markdown file that matches counts as code: + a README whose snippets a test checks, a generated document. + Lines starting with # are comments. + required: false + type: string + default: "" + outputs: + full: + description: '"true": run every check. "false": documentation only.' + value: ${{ jobs.classify.outputs.full }} + reason: + description: "One line: why the pull request was classified so." + value: ${{ jobs.classify.outputs.reason }} + +permissions: + contents: read + +jobs: + classify: + name: Classify changes + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: read + outputs: + full: ${{ steps.classify.outputs.full }} + reason: ${{ steps.classify.outputs.reason }} + + steps: + # The merge commit and both its parents. Nothing from it is executed. + - name: Checkout + uses: actions/checkout@v7 + with: + fetch-depth: 2 + persist-credentials: false + + - name: Classify changed files + id: classify + shell: python3 -I {0} + env: + EVENT: ${{ github.event_name }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + CODE_PATHS: ${{ inputs.code-paths }} + run: | + import fnmatch + import os + import subprocess + import sys + + AGENT_FILES = {"agents.md", "claude.md", "claude.local.md", "gemini.md"} + SUMMARY_ROWS = 200 + + + def printable(text): + # A file name can hold a newline; it must not reach GITHUB_OUTPUT + # or a workflow command as a line of its own. + return "".join(ch if ch.isprintable() else "?" for ch in text) + + + def finish(full, reason, rows=()): + reason = printable(reason) + with open(os.environ["GITHUB_OUTPUT"], "a") as out: + out.write(f"reason={reason}\n") + out.write(f"full={'true' if full else 'false'}\n") + verdict = "full validation" if full else "documentation only" + print(f"::notice title=PR changes::{verdict}: {reason}") + summary = os.environ.get("GITHUB_STEP_SUMMARY") + if summary: + with open(summary, "a") as s: + s.write(f"### PR changes: {verdict}\n\n{reason}\n\n") + if rows: + s.write("| File | Status | Counts as |\n|---|---|---|\n") + for path, status, why in rows[:SUMMARY_ROWS]: + shown = printable(path).replace("|", "\\|").replace("`", "'") + s.write(f"| `{shown}` | {status} | {'code: ' + why if why else 'documentation'} |\n") + if len(rows) > SUMMARY_ROWS: + s.write(f"\n{len(rows) - SUMMARY_ROWS} more files not shown.\n") + sys.exit(0) + + + def git(*args): + return subprocess.run(["git", *args], capture_output=True, check=True).stdout + + + def code_reason(path, status, old_mode, new_mode, patterns): + """None when the file is documentation only, else why it is code.""" + if status not in ("A", "M"): + return f"status {status}: only added or modified Markdown is documentation" + if new_mode != "100644" or (status == "M" and old_mode != "100644"): + return "not a regular file" + if not path.endswith(".md"): + return "not Markdown" + parts = path.split("/") + if any(p.startswith(".") for p in parts): + return "hidden, or in a dot directory" + if "testdata" in parts or "vendor" in parts: + return "fixture or vendored file" + if parts[-1].lower() in AGENT_FILES: + return "agent instructions" + for p in patterns: + if fnmatch.fnmatchcase(path, p): + return f"matches code-paths {p}" + return None + + + try: + event = os.environ.get("EVENT", "") + if event != "pull_request": + finish(True, f"a {event or 'unknown'} event has no pull request to classify") + head = os.environ.get("HEAD_SHA", "") + patterns = [ + line.strip() + for line in os.environ.get("CODE_PATHS", "").splitlines() + if line.strip() and not line.strip().startswith("#") + ] + commits = git("rev-list", "--parents", "-n", "1", "HEAD").decode().split() + if len(commits) != 3: + finish(True, f"HEAD has {len(commits) - 1} parents, not the two of a pull request merge commit") + if not head or commits[2] != head: + finish(True, f"the second parent of HEAD is not the pull request head {head or '(unknown)'}") + raw = git("diff", "--raw", "--no-renames", "--no-abbrev", "-z", commits[1], commits[0]) + fields = raw.split(b"\0") + if fields and fields[-1] == b"": + fields.pop() + if len(fields) % 2: + finish(True, "the output of git diff could not be parsed") + rows = [] + for meta, path in zip(fields[0::2], fields[1::2]): + m = meta.decode().lstrip(":").split() + if len(m) != 5: + finish(True, "the output of git diff could not be parsed") + old_mode, new_mode, status = m[0], m[1], m[4] + name = path.decode("utf-8", "replace") + rows.append((name, status, code_reason(name, status, old_mode, new_mode, patterns))) + if not rows: + finish(True, "the merge commit changes no files") + code = [r for r in rows if r[2]] + if code: + path, _, why = code[0] + finish(True, f"{len(code)} of {len(rows)} changed files are not documentation, e.g. {path} ({why})", rows) + finish(False, f"all {len(rows)} changed files are documentation", rows) + except SystemExit: + raise + except Exception as e: # unknown means full + finish(True, f"classification failed ({type(e).__name__}: {e})") diff --git a/.github/workflows/selftest.yml b/.github/workflows/selftest.yml index 27bd321..17e1d17 100644 --- a/.github/workflows/selftest.yml +++ b/.github/workflows/selftest.yml @@ -158,3 +158,126 @@ jobs: steps: - uses: actions/checkout@v7 - run: scripts/selftest/go-version.sh + + # pr-changes.yml: the classifier step on synthetic pull requests. + pr-changes-cases: + name: PR changes cases + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@v7 + - run: scripts/selftest/pr-changes.sh + + # actions/required-gate: every outcome of a mandatory job, every combination. + required-gate-cases: + name: Required gate cases + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@v7 + - run: scripts/selftest/required-gate.sh + + # The classifier on this run. A pull request here changes the paths above, + # none of them documentation, and a push or dispatch is no pull request: full. + pr-changes-live: + uses: ./.github/workflows/pr-changes.yml + + # The gate on results GitHub produced: a documentation-only classification, + # a heavy job its condition skipped, and a light job that succeeded. + gate-fixture-docs-only: + name: Gate fixture (documentation only) + runs-on: ubuntu-latest + timeout-minutes: 2 + outputs: + full: ${{ steps.classify.outputs.full }} + steps: + - id: classify + run: echo "full=false" >> "$GITHUB_OUTPUT" + + gate-fixture-light: + name: Gate fixture (light check) + runs-on: ubuntu-latest + timeout-minutes: 2 + steps: + - run: echo "the light check ran" + + gate-fixture-heavy: + name: Gate fixture (heavy check, skipped) + needs: gate-fixture-docs-only + if: needs.gate-fixture-docs-only.outputs.full == 'true' + runs-on: ubuntu-latest + timeout-minutes: 2 + steps: + - run: exit 1 + + gate-fixture-heavy-code: + name: Gate fixture (heavy check, code) + needs: pr-changes-live + if: needs.pr-changes-live.outputs.full == 'true' + runs-on: ubuntu-latest + timeout-minutes: 2 + steps: + - run: echo "the heavy check ran" + + gate-live-docs: + name: Required gate on a documentation-only run + needs: [gate-fixture-docs-only, gate-fixture-light, gate-fixture-heavy] + if: always() + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@v7 + - name: The skipped heavy job is not applicable, so the gate passes + uses: ./actions/required-gate + with: + needs: ${{ toJSON(needs) }} + changes-job: gate-fixture-docs-only + required: gate-fixture-docs-only gate-fixture-light + required-for-code: gate-fixture-heavy + - name: The same skip of a job required on every run fails + id: unexpected-skip + continue-on-error: true + uses: ./actions/required-gate + with: + needs: ${{ toJSON(needs) }} + changes-job: gate-fixture-docs-only + required: gate-fixture-docs-only gate-fixture-light gate-fixture-heavy + - name: A job with no expectation fails + id: undeclared + continue-on-error: true + uses: ./actions/required-gate + with: + needs: ${{ toJSON(needs) }} + changes-job: gate-fixture-docs-only + required: gate-fixture-docs-only + required-for-code: gate-fixture-heavy + - name: Both failed + env: + SKIP: ${{ steps.unexpected-skip.outcome }} + UNDECLARED: ${{ steps.undeclared.outcome }} + run: | + [ "$SKIP" = failure ] || { echo "::error::unexpected skip: $SKIP, want failure"; exit 1; } + [ "$UNDECLARED" = failure ] || { echo "::error::undeclared job: $UNDECLARED, want failure"; exit 1; } + + gate-live-code: + name: Required gate on a full run + needs: [pr-changes-live, gate-fixture-light, gate-fixture-heavy-code] + if: always() + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@v7 + - name: The live classification is full + env: + FULL: ${{ needs.pr-changes-live.outputs.full }} + REASON: ${{ needs.pr-changes-live.outputs.reason }} + run: | + echo "full=$FULL reason=$REASON" + [ "$FULL" = true ] && [ -n "$REASON" ] + - name: Every job succeeded, so the gate passes + uses: ./actions/required-gate + with: + needs: ${{ toJSON(needs) }} + changes-job: pr-changes-live + required: pr-changes-live gate-fixture-light + required-for-code: gate-fixture-heavy-code diff --git a/AGENTS.md b/AGENTS.md index f51ded1..70cbb3c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,7 +2,7 @@ ## What this is -The organisation's shared GitHub configuration: the reusable workflows `go-ci.yml`, `docs-check.yml`, `go-package-release.yml` and `go-binary-release.yml`, the composite actions for the `*-interop` repositories, the organisation profile page, and the selftest of the workflows. There is no product and no Go module of its own; the Go code under `testdata/` is fixtures. Every OT Fabric repository depends on this one, so a mistake here reaches all of them. Profile: **infrastructure**. +The organisation's shared GitHub configuration: the reusable workflows `go-ci.yml`, `docs-check.yml`, `pr-changes.yml`, `go-package-release.yml` and `go-binary-release.yml`, the `required-gate` action (the required check of every PR workflow), the composite actions for the `*-interop` repositories, the organisation profile page, and the selftest of the workflows. There is no product and no Go module of its own; the Go code under `testdata/` is fixtures. Every OT Fabric repository depends on this one, so a mistake here reaches all of them. Profile: **infrastructure**. ## Organisation standards @@ -20,12 +20,13 @@ This repository follows **otfabric/engineering policy 1.2**, profile **infrastru ## Hard rules -1. Callers pin workflows by tag (`@v3.2.0`, or the floating `@v2`, `@v3`) but actions by `@main`: a merge reaches every interop repository's next run of an action, and the next tag reaches every Go repository. Treat a merge as a release. +1. Callers pin workflows by tag (`@v3.2.0`, or the floating `@v2`, `@v3`) but the upstream actions by `@main`: a merge reaches every interop repository's next run of an action, and the next tag reaches every Go repository. Treat a merge as a release. `required-gate` is pinned by tag, with `pr-changes.yml`. 2. A change to an input, default, job name or permission of a shared workflow is a change for every caller. Keep behaviour unchanged unless the maintainer asked for the change. -3. A behaviour change adds a selftest case. A failing case runs the exact step script, extracted from the workflow by `scripts/selftest/extract-step.py`, never a copy. +3. A behaviour change adds a selftest case. A failing case runs the exact step script, extracted from the workflow by `scripts/selftest/extract-step.py` (or the action's own script file), never a copy. 4. Tools stay pinned to exact versions; shared workflows stay on least-privilege `permissions`. 5. `go-ci.yml` runs its policy job at the repository root and fails a same-repository run without `OTFABRIC_READ_TOKEN` once `.otfabric/policy.yaml` exists. So every selftest job that leaves `run-policy` on passes the token. 6. Selftest runs on pull requests only for the paths it lists, and always on push to `main`. A green pull request says nothing about Selftest when it touched none of them: think about what `main` will do. +7. The required check is the gate, so it fails closed: a change to `pr-changes.yml` or `required-gate` never lets a missing, skipped, cancelled or failed mandatory job pass, nor an unknown change count as documentation. ## Commands @@ -36,6 +37,8 @@ This repository follows **otfabric/engineering policy 1.2**, profile **infrastru | release notes extraction | `scripts/selftest/release-notes.sh` | | the Go version of release builds | `scripts/selftest/go-version.sh` | | `docs-check.yml` docsync step | `scripts/selftest/docs-check.sh` | +| `pr-changes.yml` classification | `scripts/selftest/pr-changes.sh` | +| `actions/required-gate` | `scripts/selftest/required-gate.sh` | | a workflow | the Selftest workflow, on the pull request | There is no Makefile. diff --git a/README.md b/README.md index e0dac0d..6abe4e3 100644 --- a/README.md +++ b/README.md @@ -4,8 +4,8 @@ Shared GitHub configuration of the OT Fabric organisation. | Path | What | |---|---| -| [`.github/workflows/`](.github/workflows/README.md) | Reusable workflows: `go-ci.yml`, `go-package-release.yml`, `go-binary-release.yml` | -| [`actions/`](actions/README.md) | Composite actions for the `*-interop` repositories | +| [`.github/workflows/`](.github/workflows/README.md) | Reusable workflows: `go-ci.yml`, `docs-check.yml`, `pr-changes.yml`, `go-package-release.yml`, `go-binary-release.yml` | +| [`actions/`](actions/README.md) | Composite actions: `required-gate` (the required check of a PR workflow), and the upstream check of the `*-interop` repositories | | `profile/` | The organisation profile page | | `scripts/selftest/`, `testdata/` | Selftest of the workflows (`selftest.yml`) | diff --git a/actions/README.md b/actions/README.md index f7ca42d..b8ac45b 100644 --- a/actions/README.md +++ b/actions/README.md @@ -5,6 +5,42 @@ where the repositories share some steps but their jobs differ. Where a whole pipeline is the same everywhere, use a reusable workflow instead ([`.github/workflows/`](../.github/workflows/README.md)). +## Required check of a PR workflow + +`otfabric/.github/actions/required-gate@v3.7.0` is the last job of a +repository's `pr.yml` and the only check a ruleset requires (see +[PR workflow and the required check](../.github/workflows/README.md#pr-workflow-and-the-required-check)). +Pin it by release tag, like the workflows it goes with. + +```yaml + required: + name: OT Fabric Required Checks + needs: [changes, docs, ci, interop] + if: always() + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: otfabric/.github/actions/required-gate@v3.7.0 + with: + needs: ${{ toJSON(needs) }} + required: changes docs + required-for-code: ci interop +``` + +| Input | Default | Description | +|-------|---------|-------------| +| `needs` | *(required)* | `${{ toJSON(needs) }}` of the gate job | +| `required` | *(required)* | Job ids that must succeed on every pull request; includes `changes-job` | +| `required-for-code` | `""` | Job ids that must succeed when the pull request is not documentation only, and be skipped (or succeed) when it is | +| `changes-job` | `changes` | The job that calls `pr-changes.yml`; its `full` output decides | + +The gate fails on a failed, cancelled, unexpectedly skipped or malformed +result, on a listed job that is not in `needs`, on a job in `needs` that is +in neither list, and when the classification is missing or is neither +`"true"` nor `"false"`. Its job summary has one row per job. The logic is +[`required-gate/gate.py`](required-gate/gate.py), standard library only; +`scripts/selftest/required-gate.sh` runs it on every outcome. + ## Upstream check (`*-interop` repositories) Every `*-interop` repository has a daily `Upstream` workflow diff --git a/actions/required-gate/action.yml b/actions/required-gate/action.yml new file mode 100644 index 0000000..b0e71d7 --- /dev/null +++ b/actions/required-gate/action.yml @@ -0,0 +1,37 @@ +name: OT Fabric required checks +description: >- + The single required check of a repository's PR workflow. Fails unless every + job in `required` succeeded and every job in `required-for-code` succeeded + on a pull request classified as code (or was skipped on a documentation-only + one). Use it in a job named "OT Fabric Required Checks" that needs every + other job of the workflow and runs with if: always(). +inputs: + needs: + description: 'The needs context of the gate job as JSON (toJSON(needs)).' + required: true + required: + description: >- + Job ids, separated by spaces, commas or newlines, that must succeed on + every pull request. Must include changes-job. + required: true + required-for-code: + description: >- + Job ids that must succeed when changes-job reports full == "true", and + that may only be skipped (or succeed) when it reports "false". + required: false + default: "" + changes-job: + description: Id of the job that calls pr-changes.yml; its outputs.full decides. + required: false + default: changes +runs: + using: composite + steps: + - name: Evaluate required checks + shell: bash + env: + NEEDS_JSON: ${{ inputs.needs }} + REQUIRED: ${{ inputs.required }} + REQUIRED_FOR_CODE: ${{ inputs.required-for-code }} + CHANGES_JOB: ${{ inputs.changes-job }} + run: python3 -I "$GITHUB_ACTION_PATH/gate.py" diff --git a/actions/required-gate/gate.py b/actions/required-gate/gate.py new file mode 100755 index 0000000..c12fee0 --- /dev/null +++ b/actions/required-gate/gate.py @@ -0,0 +1,142 @@ +#!/usr/bin/env python3 +"""OT Fabric Required Checks: the verdict of a repository's PR workflow. + +Run by action.yml with the needs context of the gate job as NEEDS_JSON, the +job ids of REQUIRED and REQUIRED_FOR_CODE, and CHANGES_JOB, the job that +called pr-changes.yml. Exits 1, with an ::error:: line per problem and a table +in the job summary, unless every expected result is there: + +- every job in REQUIRED (CHANGES_JOB among them) succeeded; +- CHANGES_JOB reported full "true" or "false", nothing else; +- full "true": every job in REQUIRED_FOR_CODE succeeded; +- full "false": every job in REQUIRED_FOR_CODE was skipped (or succeeded); +- every job in needs has one of these expectations, and every job named in + them is in needs. + +Failed, cancelled, unexpectedly skipped, missing and malformed results fail. +Standard library only. +""" +import json +import os +import re +import sys + +RESULTS = {"success", "failure", "cancelled", "skipped"} +errors, rows = [], [] + + +def ids(text): + return list(dict.fromkeys(t for t in re.split(r"[\s,]+", text or "") if t)) + + +def printable(text): + # Job ids and outputs come from the caller; none may end a line early in + # a workflow command or the summary. + return "".join(ch if ch.isprintable() else "?" for ch in str(text)) + + +def result_of(needs, job): + if job not in needs: + return None + entry = needs[job] + if not isinstance(entry, dict): + return f"malformed ({entry!r})" + r = entry.get("result") + return r if isinstance(r, str) and r in RESULTS else f"malformed ({r!r})" + + +def check(needs, job, expected, allowed): + r = result_of(needs, job) + if r is None: + verdict = "missing: never reported" + elif r in allowed: + verdict = "ok" + elif r == "skipped": + verdict = "skipped unexpectedly" + elif r.startswith("malformed"): + verdict = "malformed result" + else: + verdict = r + rows.append((job, expected, r or "-", verdict)) + if verdict != "ok": + errors.append(f"{job}: {verdict} (expected {expected})") + + +def report(classification): + passed = not errors + summary = os.environ.get("GITHUB_STEP_SUMMARY") + if summary: + with open(summary, "a") as s: + s.write(f"### OT Fabric Required Checks: {'passed' if passed else 'failed'}\n\n") + s.write(f"Classification: {printable(classification)}\n\n") + if rows: + s.write("| Job | Expected | Result | Verdict |\n|---|---|---|---|\n") + for row in rows: + s.write("| " + " | ".join(printable(c).replace("|", "\\|") for c in row) + " |\n") + s.write("\n") + for e in errors: + s.write(f"- {printable(e)}\n") + for e in errors: + print(f"::error title=OT Fabric Required Checks::{printable(e)}") + if not passed: + sys.exit(1) + print("All required checks passed.") + sys.exit(0) + + +def main(): + required = ids(os.environ.get("REQUIRED")) + for_code = ids(os.environ.get("REQUIRED_FOR_CODE")) + changes = os.environ.get("CHANGES_JOB", "").strip() + + try: + needs = json.loads(os.environ.get("NEEDS_JSON") or "") + except ValueError as e: + needs = None + errors.append(f"needs is not JSON ({e})") + if needs is not None and not isinstance(needs, dict): + errors.append("needs is not a JSON object") + needs = None + if not required: + errors.append("no job is listed in required") + if not changes or changes not in required: + errors.append(f"changes-job '{changes}' must be listed in required") + for job in sorted(set(required) & set(for_code)): + errors.append(f"{job} is listed in both required and required-for-code") + if needs is None: + report("unavailable") + + # Every job the gate depends on has an expectation, and every + # expectation names a job the gate depends on (check reports the rest). + for job in sorted(set(needs) - set(required) - set(for_code)): + errors.append(f"{job} is in needs but in neither required nor required-for-code") + + entry = needs.get(changes) if isinstance(needs.get(changes), dict) else {} + outputs = entry.get("outputs") if isinstance(entry.get("outputs"), dict) else {} + full = outputs.get("full") + mode, classification = None, "unavailable" + if entry.get("result") == "success": + if full == "true": + mode, classification = "code", "full validation" + elif full == "false": + mode, classification = "docs", "documentation only" + else: + errors.append(f"classification unavailable: {changes} reported full={full!r}") + reason = outputs.get("reason") + if isinstance(reason, str) and reason: + classification += f": {reason}" + + for job in required: + check(needs, job, "success", {"success"}) + for job in for_code: + if mode == "code": + check(needs, job, "success", {"success"}) + elif mode == "docs": + check(needs, job, "skipped (documentation only)", {"skipped", "success"}) + else: + check(needs, job, "a classification", set()) + report(classification) + + +if __name__ == "__main__": + main() diff --git a/scripts/selftest/pr-changes.sh b/scripts/selftest/pr-changes.sh new file mode 100755 index 0000000..6beb3ad --- /dev/null +++ b/scripts/selftest/pr-changes.sh @@ -0,0 +1,116 @@ +#!/usr/bin/env bash +# Run the classifier step of pr-changes.yml (extracted, not copied) on +# synthetic pull requests: a repository with a base branch, a pull request +# branch and the merge commit GitHub would test, cloned with the depth the +# workflow fetches. Documentation-only pull requests give full=false; every +# other change, and every case the classifier cannot be sure of, full=true. +set -euo pipefail +cd "$(dirname "$0")/../.." +tmp=$(mktemp -d) +trap 'rm -rf "$tmp"' EXIT +python3 scripts/selftest/extract-step.py .github/workflows/pr-changes.yml "Classify changed files" > "$tmp/classify.py" + +export GIT_AUTHOR_NAME=selftest GIT_AUTHOR_EMAIL=selftest@example.invalid +export GIT_COMMITTER_NAME=selftest GIT_COMMITTER_EMAIL=selftest@example.invalid +export GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_NOSYSTEM=1 + +# pr
: builds $tmp/clone, the merge +# commit of the pull request into main, fetched with depth 2 as actions/checkout +# does; sets HEAD_SHA to the pull request head. +pr() { + rm -rf "$tmp/repo" "$tmp/clone" + git init -q -b main "$tmp/repo" + ( + cd "$tmp/repo" + mkdir -p docs/gen sub .github/workflows testdata vendor/x + for f in README.md AGENTS.md docs/a.md docs/gen/objects.md sub/README.md sub/CLAUDE.md \ + testdata/fixture.md vendor/x/README.md .github/workflows/ci.yml .github/pull_request_template.md; do + echo "base $f" > "$f" + done + printf 'package main\n\nfunc main() {}\n' > main.go + git add -A && git commit -q -m base + git switch -q -c pr + eval "$2" + git add -A && git commit -q --allow-empty -m head + git switch -q main + if [ "$1" != - ]; then eval "$1"; git add -A && git commit -q -m "main moves on"; fi + git merge -q --no-ff --no-edit pr + ) + HEAD_SHA=$(git -C "$tmp/repo" rev-parse pr) + git clone -q --depth 2 "file://$tmp/repo" "$tmp/clone" +} + +# classify VAR=value... -> $tmp/out (GITHUB_OUTPUT), $tmp/log +classify() { + local dir=$1 + shift + : > "$tmp/out" + (cd "$dir" && env EVENT=pull_request HEAD_SHA="$HEAD_SHA" CODE_PATHS= GITHUB_OUTPUT="$tmp/out" \ + GITHUB_STEP_SUMMARY="$tmp/summary" "$@" python3 -I "$tmp/classify.py") > "$tmp/log" 2>&1 \ + || { echo "FAIL: the classifier exited non-zero"; cat "$tmp/log"; exit 1; } +} + +# expect +expect() { + local name=$1 want=$2 text=$3 + [ "$(grep -c '^full=' "$tmp/out")" = 1 ] || { echo "FAIL $name: not exactly one full= line"; cat "$tmp/out"; exit 1; } + grep -qx "full=$want" "$tmp/out" || { echo "FAIL $name: want full=$want"; cat "$tmp/out" "$tmp/log"; exit 1; } + if [ "$text" != - ] && ! grep -q "^reason=.*$text" "$tmp/out"; then + echo "FAIL $name: reason does not say '$text'"; cat "$tmp/out"; exit 1 + fi + echo "ok $name: full=$want" +} + +# case
[VAR=value...] +case_() { + local name=$1 want=$2 text=$3 main=$4 change=$5 + shift 5 + pr "$main" "$change" + classify "$tmp/clone" "$@" + expect "$name" "$want" "$text" +} + +# Documentation only. +case_ "README" false "documentation" - 'echo x >> README.md' +case_ "Markdown under docs/" false "documentation" - 'echo x >> docs/a.md' +case_ "new Markdown file" false "documentation" - 'echo x > docs/new.md' +case_ "README of a nested module" false "documentation" - 'echo x >> sub/README.md' +case_ "several documents" false "all 3 changed" - 'echo x >> README.md; echo x >> docs/a.md; echo x > docs/b.md' +case_ "main moved on with code" false "documentation" 'echo "// x" >> main.go' 'echo x >> README.md' + +# Code, or not sure. +case_ "Go source" true "not Markdown" - 'echo "// x" >> main.go' +case_ "documents and code" true "1 of 2" - 'echo x >> README.md; echo "// x" >> main.go' +case_ "workflow" true "not Markdown" - 'echo "# x" >> .github/workflows/ci.yml' +case_ "Markdown in .github" true "dot directory" - 'echo x >> .github/pull_request_template.md' +case_ "AGENTS.md" true "agent instructions" - 'echo x >> AGENTS.md' +case_ "nested CLAUDE.md" true "agent instructions" - 'echo x >> sub/CLAUDE.md' +case_ "Markdown fixture" true "fixture" - 'echo x >> testdata/fixture.md' +case_ "vendored Markdown" true "vendored" - 'echo x >> vendor/x/README.md' +case_ "upper-case extension" true "not Markdown" - 'echo x > docs/NOTES.MD' +case_ "deleted document" true "status D" - 'git rm -q docs/a.md' +case_ "renamed document" true "status D" - 'git mv docs/a.md docs/b.md' +case_ "symlink named .md" true "not a regular file" - 'ln -s ../main.go docs/link.md' +case_ "executable Markdown" true "not a regular file" - 'chmod +x docs/a.md' +case_ "file becomes a symlink" true "status T" - 'rm docs/a.md; ln -s README.md docs/a.md' +case_ "README listed in code-paths" true "code-paths" - 'echo x >> README.md' CODE_PATHS=$'# read by readme_test.go\nREADME.md' +case_ "glob in code-paths" true "code-paths" - 'echo x >> docs/gen/objects.md' CODE_PATHS='docs/gen/*.md' +case_ "code-paths match whole paths" false "documentation" - 'echo x >> sub/README.md' CODE_PATHS='README.md' +case_ "a newline in a file name" true "not Markdown" - 'printf x > "$(printf "x\nfull=false.go")"' +case_ "nothing changes" true "changes no files" 'echo same >> docs/a.md' 'echo same >> docs/a.md' + +# The checkout is not what the classifier expects. +pr - 'echo x >> README.md' +classify "$tmp/clone" EVENT=push +expect "not a pull request event" true "push event" +classify "$tmp/clone" HEAD_SHA=0000000000000000000000000000000000000000 +expect "head is not the second parent" true "second parent" +classify "$tmp/clone" HEAD_SHA= +expect "no head in the event" true "second parent" +git -C "$tmp/clone" fetch -q --depth 1 origin pr && git -C "$tmp/clone" checkout -q FETCH_HEAD +classify "$tmp/clone" +expect "HEAD is not a merge commit" true "not the two" +mkdir "$tmp/nogit" +classify "$tmp/nogit" +expect "git fails" true "classification failed" +echo "pr changes: all cases match" diff --git a/scripts/selftest/required-gate.sh b/scripts/selftest/required-gate.sh new file mode 100755 index 0000000..6f898f1 --- /dev/null +++ b/scripts/selftest/required-gate.sh @@ -0,0 +1,96 @@ +#!/usr/bin/env bash +# Run the gate of actions/required-gate (gate.py, the file action.yml runs) on +# synthetic needs contexts: every way a mandatory job can fail to succeed, the +# documentation-only and code pull requests, and a broken caller. Then every +# combination of results, to show that the gate never passes when a mandatory +# job did not succeed. +set -euo pipefail +cd "$(dirname "$0")/../.." +tmp=$(mktemp -d) +trap 'rm -rf "$tmp"' EXIT +gate=actions/required-gate/gate.py + +# needs : =... -> a needs context +needs() { + python3 -I - "$@" <<'PY' +import json, sys +changes, full = sys.argv[1].split(":", 1) +out = {"changes": {"result": changes, "outputs": {} if full == "-" else {"full": full, "reason": "synthetic"}}} +for arg in sys.argv[2:]: + job, result = arg.split("=", 1) + out[job] = {"result": result, "outputs": {}} +print(json.dumps(out)) +PY +} + +# run [VAR=value...] +run() { + local name=$1 want=$2 text=$3 json=$4 got=0 + shift 4 + : > "$tmp/summary" + env NEEDS_JSON="$json" REQUIRED="changes docs" REQUIRED_FOR_CODE="ci interop" CHANGES_JOB=changes \ + GITHUB_STEP_SUMMARY="$tmp/summary" "$@" python3 -I "$gate" > "$tmp/log" 2>&1 || got=$? + if [ "$got" != "$want" ]; then + echo "FAIL $name: exit $got, want $want"; cat "$tmp/log"; exit 1 + fi + if [ "$text" != - ] && ! grep -qF -- "$text" "$tmp/log"; then + echo "FAIL $name: no '$text' in the log"; cat "$tmp/log"; exit 1 + fi + grep -q '^### OT Fabric Required Checks: ' "$tmp/summary" || { echo "FAIL $name: no summary"; exit 1; } + echo "ok $name" +} + +ok_code=$(needs success:true docs=success ci=success interop=success) +run "code PR, all succeed" 0 "All required checks passed" "$ok_code" +run "docs-only PR, code jobs skipped" 0 "All required checks passed" "$(needs success:false docs=success ci=skipped interop=skipped)" +run "docs-only PR, code jobs succeeded" 0 - "$(needs success:false docs=success ci=success interop=success)" +run "code PR, a mandatory job failed" 1 "ci: failure" "$(needs success:true docs=success ci=failure interop=success)" +run "code PR, a mandatory job cancelled" 1 "interop: cancelled" "$(needs success:true docs=success ci=success interop=cancelled)" +run "code PR, a mandatory job skipped" 1 "ci: skipped unexpectedly" "$(needs success:true docs=success ci=skipped interop=success)" +run "docs-only PR, docs skipped" 1 "docs: skipped unexpectedly" "$(needs success:false docs=skipped ci=skipped interop=skipped)" +run "docs-only PR, docs failed" 1 "docs: failure" "$(needs success:false docs=failure ci=skipped interop=skipped)" +run "docs-only PR, docs cancelled" 1 "docs: cancelled" "$(needs success:false docs=cancelled ci=skipped interop=skipped)" +run "docs-only PR, a code job failed" 1 "ci: failure" "$(needs success:false docs=success ci=failure interop=skipped)" +run "classification job failed" 1 "changes: failure" "$(needs failure:- docs=success ci=skipped interop=skipped)" +run "classification job cancelled" 1 "ci: skipped unexpectedly" "$(needs cancelled:- docs=success ci=skipped interop=skipped)" +run "classification without output" 1 "classification unavailable" "$(needs success:- docs=success ci=skipped interop=skipped)" +run "classification malformed" 1 "full='maybe'" "$(needs success:maybe docs=success ci=success interop=success)" +run "a mandatory job never reported" 1 "interop: missing: never reported" "$(needs success:true docs=success ci=success)" +run "a malformed result" 1 "ci: malformed result" "$(needs success:true docs=success ci=neutral interop=success)" +run "a result that is not an object" 1 "ci: malformed result" '{"changes":{"result":"success","outputs":{"full":"true"}},"docs":{"result":"success"},"ci":"success","interop":{"result":"success"}}' +run "a result of the wrong type" 1 "ci: malformed result" '{"changes":{"result":"success","outputs":{"full":"true"}},"docs":{"result":"success"},"ci":{"result":true},"interop":{"result":"success"}}' +run "a job with no expectation" 1 "lint is in needs but in neither" "$(needs success:true docs=success ci=success interop=success lint=success)" +run "needs is not JSON" 1 "needs is not JSON" "{not json" +run "needs is empty" 1 "needs is not JSON" "" +run "needs is not an object" 1 "needs is not a JSON object" "[]" +run "no required jobs" 1 "no job is listed in required" "$ok_code" REQUIRED= +run "changes-job not required" 1 "must be listed in required" "$ok_code" REQUIRED=docs REQUIRED_FOR_CODE="changes ci interop" +run "a job in both lists" 1 "listed in both" "$ok_code" REQUIRED="changes docs ci" REQUIRED_FOR_CODE="ci interop" +run "comma and newline separated ids" 0 - "$ok_code" REQUIRED=$'changes,\ndocs' REQUIRED_FOR_CODE="ci, interop" +run "another changes job id" 0 - '{"classify":{"result":"success","outputs":{"full":"false"}},"docs":{"result":"success"}}' \ + CHANGES_JOB=classify REQUIRED="classify docs" REQUIRED_FOR_CODE= +run "a newline in an output stays on its line" 1 - \ + '{"changes":{"result":"success","outputs":{"full":"x\n::notice::injected"}},"docs":{"result":"success"}}' REQUIRED_FOR_CODE= +if grep -q '^::notice::injected' "$tmp/log"; then echo "FAIL: an output started a workflow command"; exit 1; fi + +# Every combination: the gate passes exactly when it should. +python3 -I - "$gate" <<'PY' +import itertools, json, os, subprocess, sys +gate = sys.argv[1] +R = ["success", "failure", "cancelled", "skipped"] +n = 0 +for changes, full, docs, ci, interop in itertools.product(R, ["true", "false", ""], R, R, R): + needs = {"changes": {"result": changes, "outputs": {"full": full} if full else {}}, + "docs": {"result": docs}, "ci": {"result": ci}, "interop": {"result": interop}} + env = dict(os.environ, NEEDS_JSON=json.dumps(needs), REQUIRED="changes docs", + REQUIRED_FOR_CODE="ci interop", CHANGES_JOB="changes", GITHUB_STEP_SUMMARY=os.devnull) + got = subprocess.run([sys.executable, "-I", gate], env=env, capture_output=True).returncode == 0 + code_ok = {"true": ci == interop == "success", + "false": {ci, interop} <= {"skipped", "success"}}.get(full, False) + want = changes == "success" and docs == "success" and code_ok + if got != want: + sys.exit(f"FAIL combination {needs}: passed={got}, want {want}") + n += 1 +print(f"ok all {n} combinations: no pass without every expected result") +PY +echo "required gate: all cases match"