From 951a55b42e829ee7efa9e19378223d1e27249b11 Mon Sep 17 00:00:00 2001 From: Matt McKay Date: Wed, 5 Aug 2026 15:37:08 +1000 Subject: [PATCH 1/3] ci(harness): decide relevance in a gate job, not with paths: filters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prerequisite for making `Action harness: all checks` a required check on main (#116 item 5). A `paths:` filter suppresses creation of the workflow RUN, not just its jobs, so no check run is ever published for that commit and a required check sits in "Expected — waiting for status to be reported" forever. GitHub documents this directly ("Troubleshooting required status checks", Handling skipped but required checks): a workflow skipped by path filtering blocks merging, whereas a job skipped by a conditional reports Success. We saw it live — release PR #119 touched only CHANGELOG.md and GitHub reported "no checks reported on the branch"; docs PR #121 likewise ran nothing. Note #116 item 5's own suggestion — a companion workflow with negated paths — does not work: `paths-ignore` is not the complement of `paths`, so a PR touching both covered and uncovered paths fires BOTH workflows and produces two same-named check runs on one SHA. That case is the norm here, not an edge case. So: both `paths:` filters go, the workflow always runs, and a `gate` job decides relevance. Only the four root jobs take `needs: gate`; everything else already chains off them, and a job whose needs were skipped is itself skipped. The decision rule is an IGNORE list, not a cover list, and that direction is the point. For a required check the expensive mistake is a green earned by running nothing, so anything unrecognised — a new action directory, a new fixture — runs the whole harness. A cover list would invert this: forgetting to extend it would silently rubber-stamp untested action changes, where today the same omission blocks the merge. The gate also self-tests. It derives the must-always-run set from the `uses: ./` lines in the workflow itself, so a typo that widened IGNORED to swallow a real action path fails the gate closed rather than skipping the suite — which harness-summary would otherwise accept as a legitimate shape. Deriving it from the workflow means future coverage (e.g. publish-gh-pages) is protected with no second edit. Permissions are job-scoped, not workflow-scoped: on a fork pull_request the other jobs run PR-authored composite-action code via `uses: ./`, and none of them needs a pull-request scope. harness-summary is rewritten to certify both shapes — all-ran and all-skipped — to reject a vacuous empty job set, and to fail when the gate and the fan-out disagree. Verified locally before pushing, by extracting both scripts from the YAML and running them against mocked inputs: the gate over the real file lists of #119 and #121 (both correctly irrelevant), an action change, a fixture change, a workflow_dispatch, push with no before-SHA, a compare 404 from a force-push (fails open), a gh API error (fails closed), and a deliberately widened IGNORED (fails closed with the self-test error). The summary was exercised across all ten reachable shapes. Refs #116 Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/test-actions.yml | 187 ++++++++++++++++++++++++++--- CHANGELOG.md | 19 +++ CONTRIBUTING.md | 10 ++ TESTING.md | 2 +- tests/README.md | 6 +- 5 files changed, 202 insertions(+), 22 deletions(-) diff --git a/.github/workflows/test-actions.yml b/.github/workflows/test-actions.yml index 05c46f3..cba77f5 100644 --- a/.github/workflows/test-actions.yml +++ b/.github/workflows/test-actions.yml @@ -41,25 +41,20 @@ name: Test Actions +# Deliberately NO `paths:` filter on either trigger. A path filter suppresses +# creation of the workflow RUN, not just its jobs, so no check run is ever +# published for that SHA and a required `Action harness: all checks` sits in +# "Expected — waiting for status to be reported" forever. GitHub documents this +# exactly ("Troubleshooting required status checks", Handling skipped but +# required checks): a workflow skipped by path filtering leaves checks pending +# and blocks merging, whereas a job skipped by a conditional reports Success. +# So the workflow always runs, and the `gate` job below decides per-job +# relevance instead. Do not re-add `paths:` here — see CONTRIBUTING. on: pull_request: - paths: - - 'restore-jupyter-cache/**' - - 'build-jupyter-cache/**' - - 'setup-environment/**' - - 'build-lectures/**' - - '.github/workflows/test-actions.yml' - - '.github/fixtures/mini-lectures/**' push: branches: - main - paths: - - 'restore-jupyter-cache/**' - - 'build-jupyter-cache/**' - - 'setup-environment/**' - - 'build-lectures/**' - - '.github/workflows/test-actions.yml' - - '.github/fixtures/mini-lectures/**' workflow_dispatch: concurrency: @@ -73,6 +68,120 @@ env: SALT: ${{ github.run_id }}-${{ github.run_attempt }} jobs: + # ========================================================================== + # Relevance gate — replaces the `paths:` filters this workflow used to carry. + # ========================================================================== + + gate: + name: 'Action harness: relevance gate' + runs-on: ubuntu-latest + timeout-minutes: 5 + # Job-scoped, not workflow-scoped. On a fork `pull_request` the other jobs + # execute PR-authored composite-action code via `uses: ./`, and none of them + # needs a pull-request scope. Job-level grants are additive over the + # workflow level (workflow-syntax: permissions are "adjusted based on any + # configuration within the workflow file, first at the workflow level and + # then at the job level"). + permissions: + contents: read # checkout, and the compare API on push + pull-requests: read # pulls/{n}/files on pull_request + outputs: + relevant: ${{ steps.decide.outputs.relevant }} + steps: + - uses: actions/checkout@v7 + + - name: Decide whether this event touches anything the harness covers + id: decide + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + EVENT_NAME: ${{ github.event_name }} + PR_NUMBER: ${{ github.event.pull_request.number }} + PUSH_BEFORE: ${{ github.event.before }} + PUSH_AFTER: ${{ github.event.after }} + run: | + set -euo pipefail + + # An IGNORE list, not a cover list, and the direction is the whole + # point. `Action harness: all checks` is a required check, so the + # expensive mistake is a green earned by running nothing. Anything + # unrecognised — a new action directory, a new fixture, a new helper — + # therefore makes the harness RUN. Forgetting to extend this list + # costs CI minutes (free on a public repo); forgetting to extend a + # cover list would silently rubber-stamp untested action changes. + IGNORED='^(CHANGELOG|README|PLAN|TESTING|CONTRIBUTING|PROJECT-OPTIMIZE-PREVIEWS)\.md$|^LICENSE$|^\.gitignore$|^docs/|^tests/|^templates/|^\.github/(ISSUE_TEMPLATE|PULL_REQUEST_TEMPLATE|dependabot\.yml)|^[A-Za-z0-9_.-]+/README\.md$' + + run_everything() { echo "relevant=true" >> "$GITHUB_OUTPUT"; echo "$1"; exit 0; } + + # Self-test. A typo that widens IGNORED would turn a real action + # change into an all-skipped fan-out, which harness-summary accepts as + # a legitimate shape — nothing downstream could catch it, so catch it + # here and fail closed. The must-run list is read out of the harness + # itself, so future coverage (e.g. `uses: ./publish-gh-pages`) is + # protected with no second edit. + DIRS=$(grep -oE 'uses: \./[A-Za-z0-9_.-]+' .github/workflows/test-actions.yml | sed 's|uses: \./||' | sort -u) + [ -n "$DIRS" ] || { echo "::error::gate self-test: no 'uses: ./' found in the harness — refusing to decide"; exit 1; } + MUST_RUN=".github/workflows/test-actions.yml .github/fixtures/mini-lectures/environment.yml" + for d in $DIRS; do MUST_RUN="$MUST_RUN $d/action.yml"; done + for p in $MUST_RUN; do + if grep -Eq "$IGNORED" <<< "$p"; then + echo "::error::gate self-test: IGNORED matches [$p], which must always run the harness"; exit 1 + fi + done + for p in CHANGELOG.md .gitignore PLAN.md tests/README.md; do + if ! grep -Eq "$IGNORED" <<< "$p"; then + echo "::error::gate self-test: IGNORED no longer matches [$p]"; exit 1 + fi + done + echo "gate self-test passed; harness invokes: $(echo $DIRS | tr '\n' ' ')" + + FILES=$(mktemp) + case "$EVENT_NAME" in + pull_request) + # Answers exactly the question `paths:` used to answer, against + # the base repo — so it works for fork PRs, and no merge-ref + # checkout is involved to go stale. + gh api --paginate "repos/$REPO/pulls/$PR_NUMBER/files?per_page=100" \ + --jq '.[].filename' > "$FILES" + # "Responses include a maximum of 3000 files." At the cap we + # cannot prove irrelevance, so run. + [ "$(wc -l < "$FILES")" -lt 3000 ] \ + || run_everything "pull request hit the 3000-file API cap — running the whole harness" + ;; + push) + case "$PUSH_BEFORE" in + ''|0000000000000000000000000000000000000000) + run_everything "push has no usable 'before' SHA — running the whole harness" ;; + esac + # A force-push can leave `before` unreachable and the compare + # 404s. That is a "cannot tell", so fail open rather than red. + if ! COMPARE=$(gh api "repos/$REPO/compare/$PUSH_BEFORE...$PUSH_AFTER" 2>/dev/null); then + run_everything "compare $PUSH_BEFORE...$PUSH_AFTER unavailable (force-push?) — running the whole harness" + fi + # No --paginate: the file list appears only on the first page and + # covers up to 300 files for the whole comparison. Paginating + # returns more commits, never more files. + [ "$(jq '.files | length' <<< "$COMPARE")" -lt 300 ] \ + || run_everything "compare hit the 300-file cap — running the whole harness" + jq -r '.files[]?.filename' <<< "$COMPARE" > "$FILES" + ;; + *) + run_everything "event [$EVENT_NAME] is a deliberate manual trigger — running the whole harness" + ;; + esac + + echo "Changed files ($(wc -l < "$FILES")):" + sed 's/^/ /' "$FILES" + + if grep -Evq "$IGNORED" "$FILES"; then + echo "relevant=true" >> "$GITHUB_OUTPUT" + echo "these changed paths are not provably irrelevant:" + grep -Ev "$IGNORED" "$FILES" | sed 's/^/ /' + else + echo "relevant=false" >> "$GITHUB_OUTPUT" + echo "every changed path is on the ignore list — the harness does not apply" + fi + # ========================================================================== # restore-jupyter-cache — unit jobs (synthetic fixtures, no conda, seconds) # ========================================================================== @@ -81,6 +190,8 @@ jobs: name: 'restore-jupyter-cache: seed build cache (save mode, genuine miss)' runs-on: ubuntu-latest timeout-minutes: 10 + needs: gate + if: needs.gate.outputs.relevant == 'true' steps: - uses: actions/checkout@v7 @@ -185,6 +296,8 @@ jobs: name: 'restore-jupyter-cache: fail-on-miss fires on a genuine miss' runs-on: ubuntu-latest timeout-minutes: 10 + needs: gate + if: needs.gate.outputs.relevant == 'true' steps: - uses: actions/checkout@v7 @@ -214,6 +327,8 @@ jobs: name: 'restore-jupyter-cache: seed execution cache (save mode)' runs-on: ubuntu-latest timeout-minutes: 10 + needs: gate + if: needs.gate.outputs.relevant == 'true' steps: - uses: actions/checkout@v7 @@ -311,6 +426,8 @@ jobs: name: 'setup-environment: standard mode, cold conda cache' runs-on: ubuntu-latest timeout-minutes: 30 + needs: gate + if: needs.gate.outputs.relevant == 'true' steps: - uses: actions/checkout@v7 @@ -648,6 +765,7 @@ jobs: timeout-minutes: 5 if: always() needs: + - gate - unit-build-seed - unit-build-restore - unit-build-restore-save @@ -663,11 +781,42 @@ jobs: - bjc-restore-build - bjc-restore-exec steps: - - name: Fail unless every harness job succeeded + # This is the job intended to be the single required status check, so it + # has to be correct in BOTH shapes the gate can produce, and must never be + # vacuously green. Three failure modes it explicitly rejects: + # - the gate itself failed or was skipped: nothing can be certified + # - gate said relevant, but a job did not succeed + # - gate said not relevant, but a job ran anyway (the fan-out and the + # gate disagree, so one of them is wrong) + - name: Certify the harness result env: RESULTS: ${{ toJSON(needs) }} run: | - echo "$RESULTS" | jq -r 'to_entries[] | "\(.key): \(.value.result)"' - echo "$RESULTS" | jq -e 'all(.[]; .result == "success")' > /dev/null \ - || { echo "::error::One or more harness jobs did not succeed"; exit 1; } - echo "✅ action harness green" + set -euo pipefail + jq -r 'to_entries[] | "\(.key): \(.value.result)"' <<< "$RESULTS" + + GATE=$(jq -r '.gate.result // "missing"' <<< "$RESULTS") + RELEVANT=$(jq -r '.gate.outputs.relevant // ""' <<< "$RESULTS") + JOBS=$(jq -c 'del(.gate)' <<< "$RESULTS") + COUNT=$(jq 'length' <<< "$JOBS") + + [ "$GATE" = "success" ] \ + || { echo "::error::the relevance gate did not succeed (result=[$GATE]) — the harness result cannot be certified"; exit 1; } + [ "$COUNT" -gt 0 ] \ + || { echo "::error::no harness jobs in needs — this check would certify nothing"; exit 1; } + + case "$RELEVANT" in + true) + jq -e 'all(.[]; .result == "success")' <<< "$JOBS" > /dev/null \ + || { echo "::error::One or more harness jobs did not succeed"; exit 1; } + echo "✅ action harness green ($COUNT jobs)" + ;; + false) + jq -e 'all(.[]; .result == "skipped")' <<< "$JOBS" > /dev/null \ + || { echo "::error::the gate found no harness-relevant changes, but not every job skipped — the gate and the fan-out disagree"; exit 1; } + echo "✅ no harness-relevant paths changed — all $COUNT jobs skipped" + ;; + *) + echo "::error::the gate produced no relevance decision (relevant=[$RELEVANT])"; exit 1 + ;; + esac diff --git a/CHANGELOG.md b/CHANGELOG.md index 89016dd..1780e35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed +- **CI**: the action harness no longer uses `paths:` filters. A path filter suppresses creation of + the workflow *run*, not just its jobs, so no check run is ever published for that commit and a + required `Action harness: all checks` would sit "waiting for status to be reported" forever — + GitHub's own guidance is to avoid requiring workflows that can be skipped. Demonstrated live: + release PR #119 touched only `CHANGELOG.md` and GitHub reported "no checks reported on the + branch". The workflow now always runs, and a new `gate` job decides relevance per-job (a job + skipped by a conditional reports success to a required check). This is the prerequisite for + making the harness a required check on `main` — issue #116 item 5, whose suggested + `paths-ignore` companion workflow would not have worked, since `paths-ignore` is not the + complement of `paths` and a mixed PR would fire both, producing two same-named check runs. + The gate's decision rule is an **ignore** list rather than a cover list, deliberately: for a + required check the expensive mistake is a green earned by running nothing, so anything + unrecognised runs the whole harness. It also self-tests — the must-always-run set is derived + from the `uses: ./` lines in the workflow itself, so widening the ignore list to swallow + a real action path fails the gate closed instead of silently skipping the suite. `harness-summary` + now certifies both shapes (all-ran and all-skipped), rejects a vacuous empty job set, and fails + if the gate and the fan-out disagree. (#116) + ### Fixed - **build-jupyter-cache**: failure alerting **never worked in container mode**, which is the documented default. Three independent bugs sat on the same 14-line path, and because that path diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a25825c..25a44e0 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -74,6 +74,16 @@ should pin an exact `v0.x.y` tag. After the 1.0.0 release, we'll add floating ma Currently outstanding: none. Add an entry here whenever you introduce one — a workaround with no entry is one a future releaser will not find. +## CI + +**Do not add a `paths:` filter to `.github/workflows/test-actions.yml`.** A path filter suppresses +creation of the workflow *run*, so no check run is published for that commit and a required +`Action harness: all checks` waits forever — GitHub's own guidance is to avoid requiring workflows +that can be skipped. Relevance is decided by the `gate` job instead, which skips the jobs (a +skipped job reports success to a required check). If the harness should ignore a new kind of path, +extend `IGNORED` in that job; it is an ignore list, so anything unrecognised runs the harness +rather than silently passing. + ### Breaking Changes **During 0.x phase (current):** diff --git a/TESTING.md b/TESTING.md index db2b548..b06c644 100644 --- a/TESTING.md +++ b/TESTING.md @@ -22,7 +22,7 @@ ## Action-Level PR Harness (`test-actions.yml`) -`.github/workflows/test-actions.yml` tests the composite actions themselves — via `uses: ./` local paths, so it exercises **the code on the PR**, not a released ref. It runs on every PR touching a covered action, on pushes to `main`, and on manual dispatch. This is stage 1 of the two-part design in issue #100; it is the permanent form of the throwaway harness that verified the #104 fix. +`.github/workflows/test-actions.yml` tests the composite actions themselves — via `uses: ./` local paths, so it exercises **the code on the PR**, not a released ref. The workflow runs on **every** PR, on pushes to `main`, and on manual dispatch; a `gate` job then decides whether the jobs themselves apply, skipping them for changes that cannot affect the actions. It works that way because a workflow suppressed by a `paths:` filter never publishes a check run at all, which would leave a required `Action harness: all checks` pending forever. This is stage 1 of the two-part design in issue #100; it is the permanent form of the throwaway harness that verified the #104 fix. Fixtures are salted with `run_id`-`run_attempt` so cache keys are unique per run and miss assertions cannot be polluted by earlier runs. The committed fixture (`.github/fixtures/mini-lectures/`) executes a real code cell, so builds populate a genuine `_build/.jupyter_cache` — unlike the container fixture, which builds with execution off. diff --git a/tests/README.md b/tests/README.md index e9290ec..4ba1989 100644 --- a/tests/README.md +++ b/tests/README.md @@ -10,7 +10,7 @@ Test assets are currently spread across the repo, each next to what it tests. Th | Location | What | Committed? | |---|---|---| -| `.github/workflows/test-actions.yml` | The PR harness — 14 jobs exercising the cache, environment and build actions via `uses: ./` local paths (#100 stage 1) | yes | +| `.github/workflows/test-actions.yml` | The PR harness — a relevance `gate` plus 15 jobs exercising the cache, environment and build actions via `uses: ./` local paths (#100 stage 1) | yes | | `.github/fixtures/mini-lectures/` | Fixture for the harness: a two-page book with a real executed code cell, so builds populate a genuine `_build/.jupyter_cache` | yes | | `containers/quantecon/tests/` | Container smoke tests and their minimal book | yes | | `tests/local/` | Throwaway clones of real lecture repos, for manual testing | **no** — git-ignored | @@ -31,4 +31,6 @@ Put local clones **here** rather than at the repo root. A root-level `test-lectu ## Adding test infrastructure -Fixtures that a workflow consumes should stay next to that workflow (`.github/fixtures/`, `containers/*/tests/`) so the `paths:` filters keep working. Use `tests/` for tooling that spans more than one of them — a shared harness runner, cross-action integration scripts, or fixture-generation tooling. +Fixtures that a workflow consumes should stay next to that workflow (`.github/fixtures/`, `containers/*/tests/`). Use `tests/` for tooling that spans more than one of them — a shared harness runner, cross-action integration scripts, or fixture-generation tooling. + +The harness no longer uses `paths:` filters, so moving a fixture will not silently stop it triggering. Relevance is decided by the `gate` job's `IGNORED` list, which is an **ignore** list: anything it does not recognise runs the whole harness. The gate also self-tests, deriving the must-always-run set from the `uses: ./` lines in the workflow itself, so it fails closed if `IGNORED` ever grows to swallow a real action path. From cf48c331637f49e9b39d6eb22a897535a2b34510 Mon Sep 17 00:00:00 2001 From: Matt McKay Date: Wed, 5 Aug 2026 15:42:54 +1000 Subject: [PATCH 2/3] ci(harness): anchor the gate self-test to real step invocations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first CI run of this PR reported: gate self-test passed; harness invokes: build-jupyter-cache build-lectures publish-gh-pages restore-jupyter-cache setup-environment publish-gh-pages is not invoked by the harness. The unanchored grep was matching the action name out of the gate's own COMMENT, which mentioned `uses: ./publish-gh-pages` as an example of future coverage. Harmless in effect — the extra entry only enlarges the must-run set, so it fails safe — but it made the self-test assert something other than what it appears to, which is precisely the failure mode this job exists to prevent. Anchoring to the start of a line counts only real step invocations. Caught by reading the job's own output rather than by a test, which is worth noting: the self-test cannot detect this class itself. Refs #116 Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/test-actions.yml | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test-actions.yml b/.github/workflows/test-actions.yml index cba77f5..bfbf8fa 100644 --- a/.github/workflows/test-actions.yml +++ b/.github/workflows/test-actions.yml @@ -117,9 +117,15 @@ jobs: # change into an all-skipped fan-out, which harness-summary accepts as # a legitimate shape — nothing downstream could catch it, so catch it # here and fail closed. The must-run list is read out of the harness - # itself, so future coverage (e.g. `uses: ./publish-gh-pages`) is - # protected with no second edit. - DIRS=$(grep -oE 'uses: \./[A-Za-z0-9_.-]+' .github/workflows/test-actions.yml | sed 's|uses: \./||' | sort -u) + # itself, so coverage added later is protected with no second edit. + # + # Anchored to the start of a line on purpose: an unanchored match also + # picks the action name out of PROSE, so a comment mentioning another + # action would quietly enlarge the must-run set. That direction is + # harmless but it makes the self-test assert something other than what + # it appears to, which is the failure mode this whole job exists to + # avoid. Only real step invocations count. + DIRS=$(grep -oE '^[[:space:]]*uses: \./[A-Za-z0-9_.-]+' .github/workflows/test-actions.yml | sed 's|.*\./||' | sort -u) [ -n "$DIRS" ] || { echo "::error::gate self-test: no 'uses: ./' found in the harness — refusing to decide"; exit 1; } MUST_RUN=".github/workflows/test-actions.yml .github/fixtures/mini-lectures/environment.yml" for d in $DIRS; do MUST_RUN="$MUST_RUN $d/action.yml"; done From 85dbf34dbe0f017f710ad7d80a8f396e05dc40ba Mon Sep 17 00:00:00 2001 From: Matt McKay Date: Wed, 5 Aug 2026 15:58:43 +1000 Subject: [PATCH 3/3] docs(tests): correct the harness job breakdown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot review. The line claimed "a relevance gate plus 15 jobs exercising ... via uses: ./ local paths", which implies harness-summary exercises actions. It does not — it invokes nothing and only certifies the others. The real shape is the gate, 14 action-invoking jobs, and the summary. Verified against the workflow rather than counted by hand. Refs #116 Co-Authored-By: Claude Opus 5 (1M context) --- tests/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/README.md b/tests/README.md index 4ba1989..68410c3 100644 --- a/tests/README.md +++ b/tests/README.md @@ -10,7 +10,7 @@ Test assets are currently spread across the repo, each next to what it tests. Th | Location | What | Committed? | |---|---|---| -| `.github/workflows/test-actions.yml` | The PR harness — a relevance `gate` plus 15 jobs exercising the cache, environment and build actions via `uses: ./` local paths (#100 stage 1) | yes | +| `.github/workflows/test-actions.yml` | The PR harness — a relevance `gate`, 14 jobs exercising the cache, environment and build actions via `uses: ./` local paths, and a `harness-summary` certification job (#100 stage 1) | yes | | `.github/fixtures/mini-lectures/` | Fixture for the harness: a two-page book with a real executed code cell, so builds populate a genuine `_build/.jupyter_cache` | yes | | `containers/quantecon/tests/` | Container smoke tests and their minimal book | yes | | `tests/local/` | Throwaway clones of real lecture repos, for manual testing | **no** — git-ignored |