From 1969c6653b7ad37e80dce6f97418dba6bede84b3 Mon Sep 17 00:00:00 2001 From: Blair Hamilton Date: Mon, 31 Aug 2026 09:40:00 -0400 Subject: [PATCH 1/7] ci: one workflow, pre-commit first, and a single required check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every Go repo under this tree had drifted into its own arrangement of the same four ideas, so this settles on the shape sh arrived at in blairham/sh#53 and applies it everywhere. Four things change. **One workflow.** Pre-commit and tests were separate files in some repos and folded together in others; a merge gate spread over two workflows is two places to keep in sync and neither one tells you whether the branch is mergeable. Everything that gates a merge now lives in ci.yml. **Pre-commit runs for every commit that reaches GitHub** — every push to a pull request, draft or not, and every push to main. It is the only job that looks at every file rather than at Go, and a secret committed to a draft is committed. Everything else waits on it: the cheap universal checks should fail before the expensive question is asked. **Tests are not re-run on merge to main.** Branch protection requires the branch to be up to date, so for a squash merge the tree that lands on main is the tree that was already tested. Re-running it proves nothing and delays the next merge. **golangci-lint runs through `go tool`.** The action's `version:` was a second pin next to go.mod's tool block, and the two had drifted apart — some repos pinned v2.12.2, some asked for `latest`, one took the action's default. CI now runs the version go.mod pins, which is the version `make lint` runs locally. Docs-only pull requests skip the code jobs, but they skip them at the *job* level rather than through a workflow `paths:` filter: a required check that never runs reports as pending forever, not as passed, which would make a prose-only pull request unmergeable. Anything unrecognised counts as code, so when the classifier is wrong it is wrong in the direction of running the tests. The single required check is ci-ok. Requiring the jobs by name does not survive this shape — a job skipped by the prose guard never reports, and a matrix job skipped by `if:` never expands, so a policy naming "test (ubuntu-latest)" waits for it forever. --- .github/workflows/ci.yml | 218 ++++++++++++++++++++++++++------------- 1 file changed, 146 insertions(+), 72 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e58733a..f9c8870 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,116 +1,148 @@ -name: CI/CD - -env: - GO_VERSION: "1.26" +name: CI +# One workflow holds everything that gates a merge, so there is one file to +# read and one check to require. Splitting pre-commit or tests into their own +# workflow bought nothing and cost a second place to keep in sync. on: - push: - branches: [ main ] pull_request: - branches: [ main ] + # `synchronize` is every push to a branch with an open pull request, which + # is the feedback that matters. Drafts get pre-commit and nothing else: + # push freely while drafting, and `ready_for_review` starts the rest. + types: [opened, reopened, synchronize, ready_for_review] + push: + # Only the pre-commit job runs here — see its `if:`. Tests are deliberately + # not re-run after a merge: branch protection requires the branch to be up + # to date, so for a squash merge the tree that lands on main is the tree + # that was already tested, and running it again proves nothing while + # delaying the next merge. + branches: [main] + +permissions: + contents: read + +concurrency: + # A superseded push is not worth finishing; only the newest commit matters. + group: ci-${{ github.ref }} + cancel-in-progress: true jobs: - test: + # Runs for every commit that reaches GitHub — every push to a pull request, + # draft or not, and every push to main. It is the only job that looks at + # *every* file rather than at Go: trailing whitespace, licence headers, + # secrets, the toolchain pin. None of that is worth detecting changes for, + # and a secret committed to a draft is committed. + pre-commit: + name: Pre-commit runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 - - - name: Set up Go - uses: actions/setup-go@v6 + - uses: actions/setup-go@v6 with: - go-version: ${{ env.GO_VERSION }} + go-version-file: go.mod check-latest: true # setup-go's manifest lags fresh Go patch releases; resolve via go.dev - cache: true - - - name: Run tests - run: make test + - name: Cache the hook environments + # pre-commit builds an environment for a hook even when SKIP tells it not + # to run one, which cost two and a half minutes a build to install a + # linter this job then declines to use. + uses: actions/cache@v4 + with: + path: ~/.cache/pre-commit + key: pre-commit-${{ runner.os }}-${{ hashFiles('.pre-commit-config.yaml') }} + - name: Run pre-commit hooks + # The golangci hooks are NOT skipped here, unlike every other repo: + # building a `language: golang` hook is this project's product, so the + # one job that runs the action from the checkout is the wrong place to + # take the shortcut. + uses: ./ - lint: + # Gating happens inside the jobs, never with a workflow-level `paths:` + # filter: a required check that never runs reports as *pending forever* + # rather than as passed, which makes a docs-only pull request unmergeable. + # Anything unrecognised counts as code, so when this is wrong it is wrong in + # the direction of running the tests. + # Only after the cheap universal checks have passed is it worth asking the + # expensive question, so this waits on pre-commit. + changes: + name: Detect changed files + needs: pre-commit + if: github.event_name == 'pull_request' && github.event.pull_request.draft == false runs-on: ubuntu-latest + outputs: + code: ${{ steps.filter.outputs.code }} steps: - uses: actions/checkout@v6 - - - name: Set up Go - uses: actions/setup-go@v6 with: - go-version: ${{ env.GO_VERSION }} - check-latest: true # setup-go's manifest lags fresh Go patch releases; resolve via go.dev - cache: true + fetch-depth: 0 # the diff needs the base commit, not just HEAD + - id: filter + name: Is any of this code? + run: | + code=false + while IFS= read -r f; do + [ -n "$f" ] || continue + case "$f" in + docs/*|*.md|LICENSE|LICENSE.*|*.png|*.jpg|*.gif|*.svg) ;; # prose + *) code=true; break ;; + esac + done <<<"$(git diff --name-only "origin/${{ github.base_ref }}"...HEAD)" + echo "code=$code" >> "$GITHUB_OUTPUT" + echo "changed files are $([ "$code" = true ] && echo 'code — full run' || echo 'prose only — skipping the code jobs')" - - name: Run golangci-lint - uses: golangci/golangci-lint-action@v9 - with: - version: v2.12.2 - args: --timeout=10m - - pre-commit: + test: + name: Build and test + needs: changes + if: needs.changes.outputs.code == 'true' runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 - - - name: Set up Go - uses: actions/setup-go@v6 + - uses: actions/setup-go@v6 with: - go-version: ${{ env.GO_VERSION }} - check-latest: true # setup-go's manifest lags fresh Go patch releases; resolve via go.dev + go-version-file: go.mod + check-latest: true cache: true + - run: make test - - name: Install gofumpt - run: go install mvdan.cc/gofumpt - - - name: Run pre-commit hooks - uses: ./ - - build: + # `go vet` is not run separately: govet is one of the linters .golangci.yml + # enables, so this job already does it, over the same code, with the same + # analysers. `go tool` rather than golangci-lint-action, so the version CI + # runs is the one pinned in go.mod — the same one `make lint` runs locally. + # The action's own `version:` was a second pin that drifted from it. + lint: + name: Lint + needs: changes + if: needs.changes.outputs.code == 'true' runs-on: ubuntu-latest - needs: [ test, lint ] steps: - uses: actions/checkout@v6 + - uses: actions/setup-go@v6 with: - fetch-depth: 0 - - - name: Set up Go - uses: actions/setup-go@v6 - with: - go-version: ${{ env.GO_VERSION }} - check-latest: true # setup-go's manifest lags fresh Go patch releases; resolve via go.dev + go-version-file: go.mod + check-latest: true cache: true + - run: go tool golangci-lint run - - name: Build - run: make build - + # Parity is this project's entire claim, so a change that breaks it fails + # before review rather than after a merge to main. language-integration: + name: Parity with Python pre-commit + needs: changes + if: needs.changes.outputs.code == 'true' runs-on: ubuntu-latest - needs: [ test ] - # Runs on every PR. Parity is this project's entire claim, so a change that - # breaks it should fail before review, not after a merge to main. steps: - uses: actions/checkout@v6 with: fetch-depth: 0 - - - name: Set up Go - uses: actions/setup-go@v6 + - uses: actions/setup-go@v6 with: - go-version: ${{ env.GO_VERSION }} - check-latest: true # setup-go's manifest lags fresh Go patch releases; resolve via go.dev + go-version-file: go.mod + check-latest: true cache: true - - - name: Set up Python - uses: actions/setup-python@v6 + - uses: actions/setup-python@v6 with: python-version: '3.13' - - - name: Set up Node.js - uses: actions/setup-node@v6 - with: - node-version: '22' - - name: Install Python pre-commit # Pinned to the line our version number claims parity with. The harness # rejects any other line, so a bump here is a deliberate parity decision. run: pip install 'pre-commit==4.6.2' - - name: Run parity tests # PARITY_REQUIRE turns "no Python found" and "zero checks ran" from a # green skip into a failure. This job exists to make the comparison; a @@ -118,10 +150,52 @@ jobs: env: PARITY_REQUIRE: '1' run: go test -v -tags=integration -timeout=600s ./test/integration/ - - name: Upload parity report if: always() uses: actions/upload-artifact@v4 with: name: parity-report path: test/integration/parity_report.json + + # No `if:` needed: a job that `needs` a skipped job is itself skipped. + build: + name: Build + needs: [test, lint] + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + with: + fetch-depth: 0 + - uses: actions/setup-go@v6 + with: + go-version-file: go.mod + check-latest: true + cache: true + - run: make build + + # The one check branch protection requires. Requiring the jobs by name does + # not survive this workflow's shape: a job skipped by the prose guard never + # reports, and a *matrix* job skipped by `if:` never expands, so a policy + # naming "test (ubuntu-latest)" waits for it forever. A single always-running + # gate that fails when any upstream job failed survives skips, matrix + # renames and new jobs alike — when jobs change here, update this `needs` + # list and nothing else. + ci-ok: + name: CI OK + needs: [pre-commit, changes, test, lint, language-integration, build] + if: always() + runs-on: ubuntu-latest + steps: + - name: Verify no required job failed + env: + RESULTS: ${{ toJSON(needs) }} + run: | + echo "$RESULTS" + bad=$(echo "$RESULTS" | jq -r 'to_entries[] + | select(.value.result != "success" and .value.result != "skipped") + | "\(.key)=\(.value.result)"') + if [ -n "$bad" ]; then + echo "not ok: $bad" + exit 1 + fi + echo "ok" From 18e7375abeef770bcbbaa0b2fb41d23258eff2ca Mon Sep 17 00:00:00 2001 From: Blair Hamilton Date: Mon, 31 Aug 2026 09:41:24 -0400 Subject: [PATCH 2/7] ci: let the gate job report under its own id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The job carried `name: CI OK`, which is the string GitHub publishes as the check name — so branch protection would have had to require "CI OK" here and "ci-ok" in koi-shell, which already requires the id. One required check name across every repo is the entire point of the gate, so the display name goes and the id stands. --- .github/workflows/ci.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f9c8870..fce47bd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -181,7 +181,6 @@ jobs: # renames and new jobs alike — when jobs change here, update this `needs` # list and nothing else. ci-ok: - name: CI OK needs: [pre-commit, changes, test, lint, language-integration, build] if: always() runs-on: ubuntu-latest From 2acd3789094a4b14dcb28e1cfacef6cd037beab1 Mon Sep 17 00:00:00 2001 From: Blair Hamilton Date: Mon, 31 Aug 2026 09:45:39 -0400 Subject: [PATCH 3/7] ci: make the change detector fail toward running the tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes to the pre-commit and changes jobs, both found by the first run of this workflow. The detector diffed against `origin/`, a ref actions/checkout does not necessarily leave behind. When it is absent `git diff` fails and prints nothing — and an empty diff is indistinguishable from "only prose changed", so the classifier concluded prose and skipped every test. A gate whose failure mode is silently skipping the tests is worse than no gate. It now diffs against the pull request's base sha, checks the commit is actually present first, and runs everything when it is not. The pre-commit job also gets the module cache. `go-mod-tidy-repo` resolves the whole dependency graph, and doing that uncached took two and a half minutes in koi-shell and then failed outright on a transient proxy.golang.org stream error. --- .github/workflows/ci.yml | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fce47bd..055df08 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -39,7 +39,8 @@ jobs: - uses: actions/setup-go@v6 with: go-version-file: go.mod - check-latest: true # setup-go's manifest lags fresh Go patch releases; resolve via go.dev + check-latest: true + cache: true # go-mod-tidy-repo resolves the whole graph; an uncached run is slow and flaky # setup-go's manifest lags fresh Go patch releases; resolve via go.dev - name: Cache the hook environments # pre-commit builds an environment for a hook even when SKIP tells it not # to run one, which cost two and a half minutes a build to install a @@ -75,15 +76,26 @@ jobs: fetch-depth: 0 # the diff needs the base commit, not just HEAD - id: filter name: Is any of this code? + env: + BASE: ${{ github.event.pull_request.base.sha }} run: | code=false - while IFS= read -r f; do - [ -n "$f" ] || continue - case "$f" in - docs/*|*.md|LICENSE|LICENSE.*|*.png|*.jpg|*.gif|*.svg) ;; # prose - *) code=true; break ;; - esac - done <<<"$(git diff --name-only "origin/${{ github.base_ref }}"...HEAD)" + if ! git cat-file -e "$BASE" 2>/dev/null; then + # No usable base commit — a force push, or a shallow fetch that did + # not reach it. `git diff` would fail here and print nothing, and an + # empty diff is indistinguishable from "only prose changed", so this + # would silently skip every test. The safe answer is to run them. + echo "base commit $BASE is not present — running everything" + code=true + else + while IFS= read -r f; do + [ -n "$f" ] || continue + case "$f" in + docs/*|*.md|LICENSE|LICENSE.*|*.png|*.jpg|*.gif|*.svg) ;; # prose + *) code=true; break ;; + esac + done <<<"$(git diff --name-only "$BASE"...HEAD)" + fi echo "code=$code" >> "$GITHUB_OUTPUT" echo "changed files are $([ "$code" = true ] && echo 'code — full run' || echo 'prose only — skipping the code jobs')" From 37f5cff7a64f492ab9e5aa806e179c836786aa6e Mon Sep 17 00:00:00 2001 From: Blair Hamilton Date: Mon, 31 Aug 2026 10:32:39 -0400 Subject: [PATCH 4/7] ci: run everything on merge to main, not just pre-commit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The push trigger carried only the pre-commit job, on the reasoning that branch protection keeps branches up to date so the tested tree is the tree that lands. That is not quite true of a squash merge: the branch was tested before the squash, main can move under it afterwards, and the commit that ends up on main is one no pull request ever built. The merge result now gets its own run rather than inheriting the branch's green tick. Three things follow from it. The change detector runs on a push as well, diffing against `github.event.before` — the commit main pointed at — so the diff is the merge itself. It still treats a missing base commit as "run everything". Drafts keep skipping the code jobs; a push to main never does. Concurrency no longer cancels in-progress runs on main. A superseded push to a pull request is not worth finishing, but a run on main is the record of whether that merge was good, and cancelling it leaves the question open. --- .github/workflows/ci.yml | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 055df08..24de876 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,20 +10,21 @@ on: # push freely while drafting, and `ready_for_review` starts the rest. types: [opened, reopened, synchronize, ready_for_review] push: - # Only the pre-commit job runs here — see its `if:`. Tests are deliberately - # not re-run after a merge: branch protection requires the branch to be up - # to date, so for a squash merge the tree that lands on main is the tree - # that was already tested, and running it again proves nothing while - # delaying the next merge. + # Everything runs here too, not just pre-commit. A squash merge lands a + # tree no pull request ever built — the branch was tested before the + # squash, and `main` may have moved under it since — so the merge result + # gets its own run rather than inheriting the branch's green tick. branches: [main] permissions: contents: read concurrency: - # A superseded push is not worth finishing; only the newest commit matters. + # A superseded push to a pull request is not worth finishing. A push to main + # is: it is the record of whether that merge was good, and cancelling it + # leaves the question unanswered. group: ci-${{ github.ref }} - cancel-in-progress: true + cancel-in-progress: ${{ github.event_name == 'pull_request' }} jobs: # Runs for every commit that reaches GitHub — every push to a pull request, @@ -66,7 +67,8 @@ jobs: changes: name: Detect changed files needs: pre-commit - if: github.event_name == 'pull_request' && github.event.pull_request.draft == false + # Drafts still skip the code jobs; a push to main never does. + if: github.event_name == 'push' || github.event.pull_request.draft == false runs-on: ubuntu-latest outputs: code: ${{ steps.filter.outputs.code }} @@ -77,7 +79,9 @@ jobs: - id: filter name: Is any of this code? env: - BASE: ${{ github.event.pull_request.base.sha }} + # On a merge to main there is no pull request; `before` is the commit + # main pointed at, which makes the diff the merge itself. + BASE: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || github.event.before }} run: | code=false if ! git cat-file -e "$BASE" 2>/dev/null; then From abed21351c8db4d557cb55f824bc61a69905d913 Mon Sep 17 00:00:00 2001 From: Blair Hamilton Date: Mon, 31 Aug 2026 11:14:34 -0400 Subject: [PATCH 5/7] ci: lint the change, in one place, and make it actually run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Linting was in two places with two different jobs to do, and the CI half was about to become a no-op. This settles it: the hook reports your change, the Lint job reports the pull request's change, and nothing lints the whole backlog on every commit. The hook stays `golangci-lint` — the `--new-from-rev HEAD` variant. It runs on every commit and should report what you just wrote, not the repository's history. Measured, the choice costs nothing either way: `pass_filenames` is false, so golangci type-checks the whole module regardless and `--new-from-rev` only filters what it prints — 0.82s against 0.93s on a warm cache. The reason to prefer it is the signal, not the second. `--new-from-rev HEAD` is useless on a runner, though: it compares the working tree against HEAD, and a CI checkout is clean, so it reports nothing while looking configured. So CI skips that one hook and the Lint job does the equivalent properly, diffing from the base *commit* — the pull request's base sha, or on a merge the commit main pointed at. Not `origin/`: actions/checkout does not reliably leave that ref behind. A missing base commit falls back to linting everything, because linting nothing is the one outcome worth ruling out. `golangci-lint-fmt` is no longer skipped in CI. Formatting is absolute rather than relative to a diff, and it is the same second of work. --- .github/workflows/ci.yml | 94 ++++++++++++++++++++++++---------------- .pre-commit-config.yaml | 10 +++++ 2 files changed, 66 insertions(+), 38 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 24de876..7c28e6a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,8 +6,9 @@ name: CI on: pull_request: # `synchronize` is every push to a branch with an open pull request, which - # is the feedback that matters. Drafts get pre-commit and nothing else: - # push freely while drafting, and `ready_for_review` starts the rest. + # is the feedback that matters. A draft is treated exactly like any other + # pull request: the distinction bought a little runner time and cost a + # second set of rules to hold in your head. types: [opened, reopened, synchronize, ready_for_review] push: # Everything runs here too, not just pre-commit. A squash merge lands a @@ -41,20 +42,14 @@ jobs: with: go-version-file: go.mod check-latest: true - cache: true # go-mod-tidy-repo resolves the whole graph; an uncached run is slow and flaky # setup-go's manifest lags fresh Go patch releases; resolve via go.dev - - name: Cache the hook environments - # pre-commit builds an environment for a hook even when SKIP tells it not - # to run one, which cost two and a half minutes a build to install a - # linter this job then declines to use. - uses: actions/cache@v4 - with: - path: ~/.cache/pre-commit - key: pre-commit-${{ runner.os }}-${{ hashFiles('.pre-commit-config.yaml') }} + cache: true # go-mod-tidy-repo resolves the whole graph; uncached it is slow and flaky - name: Run pre-commit hooks - # The golangci hooks are NOT skipped here, unlike every other repo: - # building a `language: golang` hook is this project's product, so the - # one job that runs the action from the checkout is the wrong place to - # take the shortcut. + # Nothing is skipped here, unlike every other repo, which skips the + # HEAD-relative `golangci-lint`. Building a `language: golang` hook is + # this project's product, and the hook reporting nothing against a clean + # checkout is beside the point — that it *builds and runs* is the thing + # under test, and this is the one job that runs the action from the + # checkout. The Lint job still gates the change itself. uses: ./ # Gating happens inside the jobs, never with a workflow-level `paths:` @@ -67,8 +62,6 @@ jobs: changes: name: Detect changed files needs: pre-commit - # Drafts still skip the code jobs; a push to main never does. - if: github.event_name == 'push' || github.event.pull_request.draft == false runs-on: ubuntu-latest outputs: code: ${{ steps.filter.outputs.code }} @@ -84,7 +77,14 @@ jobs: BASE: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || github.event.before }} run: | code=false - if ! git cat-file -e "$BASE" 2>/dev/null; then + if [ "$GITHUB_EVENT_NAME" != pull_request ]; then + # A merge runs everything. Not because the merge itself changed code + # — it may be pure prose — but because this is the first time this + # tree is tested *as main*, and what it protects against is an + # earlier merge that broke something nobody has run since. + echo "push to $GITHUB_REF_NAME — running everything" + code=true + elif ! git cat-file -e "$BASE" 2>/dev/null; then # No usable base commit — a force push, or a shallow fetch that did # not reach it. `git diff` would fail here and print nothing, and an # empty diff is indistinguishable from "only prose changed", so this @@ -117,24 +117,6 @@ jobs: cache: true - run: make test - # `go vet` is not run separately: govet is one of the linters .golangci.yml - # enables, so this job already does it, over the same code, with the same - # analysers. `go tool` rather than golangci-lint-action, so the version CI - # runs is the one pinned in go.mod — the same one `make lint` runs locally. - # The action's own `version:` was a second pin that drifted from it. - lint: - name: Lint - needs: changes - if: needs.changes.outputs.code == 'true' - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v6 - - uses: actions/setup-go@v6 - with: - go-version-file: go.mod - check-latest: true - cache: true - - run: go tool golangci-lint run # Parity is this project's entire claim, so a change that breaks it fails # before review rather than after a merge to main. @@ -176,7 +158,7 @@ jobs: # No `if:` needed: a job that `needs` a skipped job is itself skipped. build: name: Build - needs: [test, lint] + needs: [test] runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 @@ -189,6 +171,42 @@ jobs: cache: true - run: make build + # "Only the changes", done in a way that works on a runner. The hook's + # `--new-from-rev HEAD` is a no-op against a clean checkout, so this diffs + # from the base *commit*: the pull request's base sha, or on a merge the + # commit main pointed at. `origin/` is deliberately not used — + # actions/checkout does not reliably leave that ref behind. + # + # `go vet` is not run separately: govet is one of the linters .golangci.yml + # enables, so this job already does it over the same code. + lint: + name: Lint + needs: changes + if: needs.changes.outputs.code == 'true' + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + with: + fetch-depth: 0 # --new-from-rev needs the base commit, not just HEAD + - uses: actions/setup-go@v6 + with: + go-version-file: go.mod + check-latest: true + cache: true + - name: Lint what this change introduced + env: + BASE: ${{ github.event.pull_request.base.sha || github.event.before }} + run: | + LINT="go tool golangci-lint" + if git cat-file -e "$BASE" 2>/dev/null; then + $LINT run --new-from-rev "$BASE" + else + # No usable base commit, so there is no diff to be relative to. + # Lint everything rather than silently lint nothing. + echo "base commit $BASE is not present — linting everything" + $LINT run + fi + # The one check branch protection requires. Requiring the jobs by name does # not survive this workflow's shape: a job skipped by the prose guard never # reports, and a *matrix* job skipped by `if:` never expands, so a policy @@ -197,7 +215,7 @@ jobs: # renames and new jobs alike — when jobs change here, update this `needs` # list and nothing else. ci-ok: - needs: [pre-commit, changes, test, lint, language-integration, build] + needs: [pre-commit, changes, test, language-integration, build, lint] if: always() runs-on: ubuntu-latest steps: diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index de5133e..0d62148 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -42,6 +42,16 @@ repos: hooks: - id: golangci-lint-fmt + # The diff-scoped hook (`--new-from-rev HEAD`), deliberately: this runs on + + # every commit and should report what you just wrote, not the + + # repository's whole backlog. CI lints the change against its base. + + # + + # Keep `rev` in lockstep with go.mod's tool pin. + - id: golangci-lint # Secret scanning - repo: https://github.com/gitleaks/gitleaks From b916c5d2ed5401b56a386ae015a4d62a446550c8 Mon Sep 17 00:00:00 2001 From: Blair Hamilton Date: Mon, 31 Aug 2026 11:22:11 -0400 Subject: [PATCH 6/7] ci: pre-commit is the linter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes the separate lint job and the SKIP that suppressed the lint hooks. golangci-lint runs in pre-commit, on every commit, reporting what the change introduced — one place, one configuration, no second pin to keep in step. --- .github/workflows/ci.yml | 38 +------------------------------------- 1 file changed, 1 insertion(+), 37 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7c28e6a..c29922b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -171,42 +171,6 @@ jobs: cache: true - run: make build - # "Only the changes", done in a way that works on a runner. The hook's - # `--new-from-rev HEAD` is a no-op against a clean checkout, so this diffs - # from the base *commit*: the pull request's base sha, or on a merge the - # commit main pointed at. `origin/` is deliberately not used — - # actions/checkout does not reliably leave that ref behind. - # - # `go vet` is not run separately: govet is one of the linters .golangci.yml - # enables, so this job already does it over the same code. - lint: - name: Lint - needs: changes - if: needs.changes.outputs.code == 'true' - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v6 - with: - fetch-depth: 0 # --new-from-rev needs the base commit, not just HEAD - - uses: actions/setup-go@v6 - with: - go-version-file: go.mod - check-latest: true - cache: true - - name: Lint what this change introduced - env: - BASE: ${{ github.event.pull_request.base.sha || github.event.before }} - run: | - LINT="go tool golangci-lint" - if git cat-file -e "$BASE" 2>/dev/null; then - $LINT run --new-from-rev "$BASE" - else - # No usable base commit, so there is no diff to be relative to. - # Lint everything rather than silently lint nothing. - echo "base commit $BASE is not present — linting everything" - $LINT run - fi - # The one check branch protection requires. Requiring the jobs by name does # not survive this workflow's shape: a job skipped by the prose guard never # reports, and a *matrix* job skipped by `if:` never expands, so a policy @@ -215,7 +179,7 @@ jobs: # renames and new jobs alike — when jobs change here, update this `needs` # list and nothing else. ci-ok: - needs: [pre-commit, changes, test, language-integration, build, lint] + needs: [pre-commit, changes, test, language-integration, build] if: always() runs-on: ubuntu-latest steps: From dda4c30973311523d3db53d8ae13f3ffd81c4dff Mon Sep 17 00:00:00 2001 From: Blair Hamilton Date: Mon, 31 Aug 2026 11:35:38 -0400 Subject: [PATCH 7/7] ci: drop the aggregate gate, match sh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes the ci-ok job and gates per step rather than per job, which is the shape sh settled on. The gate existed because jobs were skipped wholesale on a prose-only change, and a skipped job never reports — so branch protection could not name the jobs themselves. Gating inside the job removes that constraint: the job always runs and always reports, and only the expensive steps are skipped. With that, the branch policy names the jobs directly and there is nothing to keep in step with a `needs` list. Every job that reads the detector now lists `changes` in its `needs`, since the output has to be reachable from the step conditions. Concurrency cancels in-progress runs again, on main as well as on pull requests, matching sh rather than carrying a local exception. --- .github/workflows/ci.yml | 47 ++++++++++++---------------------------- 1 file changed, 14 insertions(+), 33 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c29922b..92640d8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,11 +21,9 @@ permissions: contents: read concurrency: - # A superseded push to a pull request is not worth finishing. A push to main - # is: it is the record of whether that merge was good, and cancelling it - # leaves the question unanswered. + # A superseded push is not worth finishing; only the newest commit matters. group: ci-${{ github.ref }} - cancel-in-progress: ${{ github.event_name == 'pull_request' }} + cancel-in-progress: true jobs: # Runs for every commit that reaches GitHub — every push to a pull request, @@ -106,16 +104,18 @@ jobs: test: name: Build and test needs: changes - if: needs.changes.outputs.code == 'true' runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 + if: needs.changes.outputs.code == 'true' - uses: actions/setup-go@v6 + if: needs.changes.outputs.code == 'true' with: go-version-file: go.mod check-latest: true cache: true - run: make test + if: needs.changes.outputs.code == 'true' # Parity is this project's entire claim, so a change that breaks it fails @@ -123,25 +123,29 @@ jobs: language-integration: name: Parity with Python pre-commit needs: changes - if: needs.changes.outputs.code == 'true' runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 + if: needs.changes.outputs.code == 'true' with: fetch-depth: 0 - uses: actions/setup-go@v6 + if: needs.changes.outputs.code == 'true' with: go-version-file: go.mod check-latest: true cache: true - uses: actions/setup-python@v6 + if: needs.changes.outputs.code == 'true' with: python-version: '3.13' - name: Install Python pre-commit + if: needs.changes.outputs.code == 'true' # Pinned to the line our version number claims parity with. The harness # rejects any other line, so a bump here is a deliberate parity decision. run: pip install 'pre-commit==4.6.2' - name: Run parity tests + if: needs.changes.outputs.code == 'true' # PARITY_REQUIRE turns "no Python found" and "zero checks ran" from a # green skip into a failure. This job exists to make the comparison; a # run that makes no comparison has not done its job. @@ -158,41 +162,18 @@ jobs: # No `if:` needed: a job that `needs` a skipped job is itself skipped. build: name: Build - needs: [test] + needs: [changes, test] runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 + if: needs.changes.outputs.code == 'true' with: fetch-depth: 0 - uses: actions/setup-go@v6 + if: needs.changes.outputs.code == 'true' with: go-version-file: go.mod check-latest: true cache: true - run: make build - - # The one check branch protection requires. Requiring the jobs by name does - # not survive this workflow's shape: a job skipped by the prose guard never - # reports, and a *matrix* job skipped by `if:` never expands, so a policy - # naming "test (ubuntu-latest)" waits for it forever. A single always-running - # gate that fails when any upstream job failed survives skips, matrix - # renames and new jobs alike — when jobs change here, update this `needs` - # list and nothing else. - ci-ok: - needs: [pre-commit, changes, test, language-integration, build] - if: always() - runs-on: ubuntu-latest - steps: - - name: Verify no required job failed - env: - RESULTS: ${{ toJSON(needs) }} - run: | - echo "$RESULTS" - bad=$(echo "$RESULTS" | jq -r 'to_entries[] - | select(.value.result != "success" and .value.result != "skipped") - | "\(.key)=\(.value.result)"') - if [ -n "$bad" ]; then - echo "not ok: $bad" - exit 1 - fi - echo "ok" + if: needs.changes.outputs.code == 'true'