diff --git a/.gitea/workflows/docker-build.yml b/.gitea/workflows/docker-build.yml index 45278c9dd..f98fe7a70 100644 --- a/.gitea/workflows/docker-build.yml +++ b/.gitea/workflows/docker-build.yml @@ -36,6 +36,14 @@ name: Build ErsatzTV Image # `if:`-skip a required job: on Gitea 1.25.4 a skipped job reports commit-status state `skipped` # (verified, throwaway PR #418) and we don't rely on how branch protection treats a skipped # REQUIRED context. See docs/ci-cd.md -> "Docs-only skip". +# +# ALREADY-VALIDATED SKIP (ersatztv#420): a second, sibling gate in `test`, `migrations` and +# `functional-e2e` only (NOT `build`). On a push-to-main merge commit, `id: revalidate` runs +# `scripts/ci-detect-already-validated.sh`, which emits `skip=true` only when the merged tree is +# byte-identical to a PR head that already has a green Gitea combined status — i.e. the exact +# source was already validated in the PR run. Every heavy step in those three jobs additionally +# gates on `steps.revalidate.outputs.skip != 'true'`. `build` is untouched and always runs on +# main, so the image is still built (from already-validated source) even when the skip fires. on: workflow_dispatch: @@ -103,9 +111,9 @@ jobs: - name: Checkout uses: actions/checkout@v4 with: - # only the test job's steps below need the working tree; git history/tags - # are only needed by the `build` job's `git describe` (ersatztv#190) - fetch-depth: 1 + # git history/tags are needed by the `build` job's `git describe` (ersatztv#190) and, + # here, by the #420 revalidate step's `HEAD^2` tree comparison on a main merge commit. + fetch-depth: 2 # ersatztv#416: is this a docs-only change? If so, every heavy step below is skipped and this # REQUIRED job reports success in seconds. It still RUNS (never `if:`-skipped) so the required @@ -113,9 +121,14 @@ jobs: - name: Detect docs-only changes id: detect run: scripts/ci-detect-docs-only.sh + - name: Detect already-validated tree (#420) + id: revalidate + env: + ETV_STATUS_AUTH: ${{ secrets.REGISTRY_USER }}:${{ secrets.REGISTRY_PASSWORD }} + run: scripts/ci-detect-already-validated.sh - name: Cache NuGet packages - if: steps.detect.outputs.docs_only != 'true' + if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true' uses: actions/cache@v4 with: path: ~/.nuget/packages @@ -123,13 +136,13 @@ jobs: restore-keys: nuget-${{ runner.os }}- - name: Restore - if: steps.detect.outputs.docs_only != 'true' + if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true' run: dotnet restore # Replaces setup-node's built-in `cache: npm`. The toolchain image supplies node/npm, but # the SPA's package downloads are project deps, so they stay cached per lockfile. - name: Cache npm packages - if: steps.detect.outputs.docs_only != 'true' + if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true' uses: actions/cache@v4 with: path: ~/.npm @@ -137,45 +150,45 @@ jobs: restore-keys: npm-${{ runner.os }}- - name: Install SPA dependencies - if: steps.detect.outputs.docs_only != 'true' + if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true' working-directory: web run: npm ci - name: Check generated SPA API client - if: steps.detect.outputs.docs_only != 'true' + if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true' working-directory: web run: npm run check:api - name: Lint SPA - if: steps.detect.outputs.docs_only != 'true' + if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true' working-directory: web run: npm run lint - name: Typecheck SPA - if: steps.detect.outputs.docs_only != 'true' + if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true' working-directory: web run: npm run typecheck - name: Test SPA - if: steps.detect.outputs.docs_only != 'true' + if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true' working-directory: web run: npm test -- --run - name: Build SPA - if: steps.detect.outputs.docs_only != 'true' + if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true' working-directory: web run: npm run build - name: Strip Scanner project ref (matches Docker build) - if: steps.detect.outputs.docs_only != 'true' + if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true' run: sed -i '/Scanner/d' ErsatzTV/ErsatzTV.csproj - name: Build - if: steps.detect.outputs.docs_only != 'true' + if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true' run: dotnet build --configuration Release --no-restore - name: Test - if: steps.detect.outputs.docs_only != 'true' + if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true' run: >- dotnet test --configuration Release --no-build --blame-hang-timeout "2m" --verbosity normal --collect:"XPlat Code Coverage" --settings coverlet.runsettings --results-directory ./coverage @@ -186,7 +199,7 @@ jobs: # floor later"), so this step is purely informational — continue-on-error keeps a missing # report or a transient tool-install failure from ever blocking a build. - name: Coverage summary - if: steps.detect.outputs.docs_only != 'true' + if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true' continue-on-error: true run: | set -euo pipefail @@ -239,7 +252,7 @@ jobs: # `defaults.run.shell: bash` means `-e -o pipefail` is on, so a failed `cat`/redirect here # would redden a green test job. `continue-on-error` is what actually makes it advisory, # the same guarantee the Coverage summary step above uses. - if: ${{ always() && steps.detect.outputs.docs_only != 'true' }} + if: ${{ always() && steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true' }} continue-on-error: true run: | mib() { echo "$(( ${1:-0} / 1048576 ))"; } @@ -339,17 +352,24 @@ jobs: steps: - name: Checkout uses: actions/checkout@v4 - # default fetch-depth: 1 -- this job never runs git describe/log, only - # actions/checkout@v4's default (shallow) history is needed (ersatztv#190) + with: + # was the default fetch-depth: 1 (ersatztv#190); bumped to 2 so the #420 revalidate + # step's `HEAD^2` tree comparison can resolve on a main merge commit. + fetch-depth: 2 # ersatztv#416: docs-only? Skip the build + migration replay; the job still reports success in # seconds. REQUIRED context, so it always RUNS (never `if:`-skipped). See the workflow header. - name: Detect docs-only changes id: detect run: scripts/ci-detect-docs-only.sh + - name: Detect already-validated tree (#420) + id: revalidate + env: + ETV_STATUS_AUTH: ${{ secrets.REGISTRY_USER }}:${{ secrets.REGISTRY_PASSWORD }} + run: scripts/ci-detect-already-validated.sh - name: Cache NuGet packages - if: steps.detect.outputs.docs_only != 'true' + if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true' uses: actions/cache@v4 with: path: ~/.nuget/packages @@ -357,11 +377,11 @@ jobs: restore-keys: nuget-${{ runner.os }}- - name: Restore - if: steps.detect.outputs.docs_only != 'true' + if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true' run: dotnet restore - name: Build - if: steps.detect.outputs.docs_only != 'true' + if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true' run: dotnet build --configuration Release --no-restore # dotnet-ef is baked into the CI toolchain image (docker/ci/Dockerfile) and already on PATH @@ -369,7 +389,7 @@ jobs: # SQLite is the prod provider; both checks validated locally. - name: SQLite — model drift + apply all migrations to a fresh DB - if: steps.detect.outputs.docs_only != 'true' + if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true' run: | set -euo pipefail echo "::group::SQLite model drift (has-pending-model-changes)" @@ -385,7 +405,7 @@ jobs: # MySql uses ServerVersion.AutoDetect (connects at config time), so it runs against the # service container above. MySql__ConnectionString maps to config key "MySql:ConnectionString". - name: MySql — model drift + apply all migrations to a fresh DB - if: steps.detect.outputs.docs_only != 'true' + if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true' env: # DefaultCommandTimeout is raised from MySqlConnector's 30s default: replaying every # migration to a fresh DB issues DDL commands that can exceed 30s when two migration jobs @@ -440,15 +460,22 @@ jobs: - name: Checkout uses: actions/checkout@v4 with: - fetch-depth: 1 + # bumped from 1 (ersatztv#190 default) so the #420 revalidate step's `HEAD^2` tree + # comparison can resolve on a main merge commit. + fetch-depth: 2 # ersatztv#416: docs-only? Skip the boot + curl harness (advisory job; safe to no-op). - name: Detect docs-only changes id: detect run: scripts/ci-detect-docs-only.sh + - name: Detect already-validated tree (#420) + id: revalidate + env: + ETV_STATUS_AUTH: ${{ secrets.REGISTRY_USER }}:${{ secrets.REGISTRY_PASSWORD }} + run: scripts/ci-detect-already-validated.sh - name: Cache NuGet packages - if: steps.detect.outputs.docs_only != 'true' + if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true' uses: actions/cache@v4 with: path: ~/.nuget/packages @@ -456,11 +483,11 @@ jobs: restore-keys: nuget-${{ runner.os }}- - name: Restore - if: steps.detect.outputs.docs_only != 'true' + if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true' run: dotnet restore - name: Cache npm packages - if: steps.detect.outputs.docs_only != 'true' + if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true' uses: actions/cache@v4 with: path: ~/.npm @@ -468,17 +495,17 @@ jobs: restore-keys: npm-${{ runner.os }}- - name: Install SPA dependencies - if: steps.detect.outputs.docs_only != 'true' + if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true' working-directory: web run: npm ci - name: Build SPA - if: steps.detect.outputs.docs_only != 'true' + if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true' working-directory: web run: npm run build - name: Build (Release) - if: steps.detect.outputs.docs_only != 'true' + if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true' run: dotnet build ErsatzTV.sln --configuration Release --no-restore # The old `command -v ffmpeg || sudo apt-get install ffmpeg` step is gone (ersatztv#390): @@ -489,7 +516,7 @@ jobs: # the image, so still no per-run install. The scan flow self-skips if ffmpeg is ever absent. - name: Boot instance and run functional-E2E harness - if: steps.detect.outputs.docs_only != 'true' + if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true' run: | set -euo pipefail export ETV_BUILD_CONFIG=Release ETV_UI_PORT=8409 diff --git a/docs/ci-cd.md b/docs/ci-cd.md index c011d2ed0..c3ced0597 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -369,9 +369,55 @@ API path / no `.cs` changed → they pass in ~5s), so they needed no change. `do `decisions-guard` and `ci-image-pin` keep running on docs-only changes — the first two are *about* docs and must. -Not in scope: the separate redundancy of running the **whole matrix on a PR and again on the -merge-to-`main`** over identical code (ersatztv#420), and the within-run triple `dotnet build` -(ersatztv#398). +Not in scope: the within-run triple `dotnet build` (ersatztv#398; measured and rejected as +build-once — see `docs/decisions.md`). The separate redundancy of running the **whole matrix on a +PR and again on the merge-to-`main`** over identical code (ersatztv#420) is addressed below. + +### Cross-run tree-identity skip (ersatztv#420) + +A merge to `main` re-runs the entire matrix over code the PR's last run already validated — the +same redundancy as docs-only, but for identical code rather than docs. On a `push` to `main` that +is a real merge commit, `test`/`migrations`/`functional-e2e` each run +`scripts/ci-detect-already-validated.sh` as a **second** detect step (`id: revalidate`, right after +the docs-only detect), and every heavy step gains an added `&& steps.revalidate.outputs.skip != +'true'` to its existing `if:`. + +**Skip condition — all four required, else fail-safe `skip=false`:** + +- the event is a push to `refs/heads/main`; +- `HEAD` has a second parent `HEAD^2` (a real merge commit — the PR head CI already validated; + squash, rebase, fast-forward, or a direct push have no `HEAD^2`, so they run); +- `git rev-parse HEAD^{tree}` equals `HEAD^2^{tree}` — main did not advance since the PR's last run, + a byte-identical tree; +- `HEAD^2` has a **green Gitea combined commit status**, queried via the API with + `ETV_STATUS_AUTH`. Trusting the aggregate `.state` is sound: a `skipped` context does **not** drag + the combined state below `success` (verified live against this instance — a real merge commit with + four `skipped` PR-only contexts still reported `.state == success`), and the two required jobs + never report `skipped` (they always run and report a real `success`/`failure`), so + `.state == success` implies they were green. + +Those three jobs check out `fetch-depth: 2` so `HEAD^2` and its tree resolve. + +**Why it's safe: `build` is not gated.** The three heavy jobs skip their *steps* (same +required-context reasoning as docs-only — they still run and report `success` in seconds), but +**`build` always runs on `main`, ungated**, building and pushing the image from that identical, +already-validated tree. No image ships from unvalidated source. The required contexts are +unchanged (`Build & test (.NET)`, `EF migration integrity (SQLite + MySql)`) — no branch-protection +change. + +**Fail-safe bias.** Any uncertainty — not a main push, no `HEAD^2`, a differing tree, a +missing/failing/non-`success` status, missing auth — resolves to `skip=false` and runs the full +matrix. A false skip could ship an under-validated image, so every ambiguous case runs everything. + +**Honest limitation — this fires rarely here, by design.** The tree is identical only on a +*fast-forward-equivalent* merge: main did not advance since the PR's last green run **and** the PR +head was not rebased at merge time. Two routine patterns defeat it in this repo: (1) under parallel +merges main usually advances; and (2) — the bigger one — the standard workflow **rebases a PR +before merging** to resolve the append-only `docs/decisions.md` conflict (see MEMORY: the +"decisions.md conflict treadmill"), which mints a new head SHA whose tree was never itself +CI-validated, so the tree-match check correctly declines. So the skip is a genuine but *occasional* +win (clean, up-to-date, un-rebased merges in quiet periods) — correct-but-conservative by +construction, not a general dedup. It never fires unsafely; when in doubt it runs the full matrix. ### `docs-reminder` job (non-blocking, PR-only) diff --git a/docs/decisions.md b/docs/decisions.md index 716c2be07..ebb246bca 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -97,6 +97,7 @@ in-file entries. - [2026-07-18 — Auto-Tune per-source weights ride #70's MultiCollection machinery; created at tune time, not a post-hoc PUT (#425)](#2026-07-18--auto-tune-per-source-weights-ride-70s-multicollection-machinery-created-at-tune-time-not-a-post-hoc-put-425) - [2026-07-18 — Search all-items is paged to cap DoS exposure; SPA add-all pages to completeness (#293)](#2026-07-18--search-all-items-is-paged-to-cap-dos-exposure-spa-add-all-pages-to-completeness-293) - [2026-07-18 — Unsupported PlaybackOrder is loud at build time; a declared support matrix and tripwire test make new orders safe by construction (#403)](#2026-07-18--unsupported-playbackorder-is-loud-at-build-time-a-declared-support-matrix-and-tripwire-test-make-new-orders-safe-by-construction-403) +- [2026-07-18 — CI build-once was measured and rejected; keep the #420 tree-skip](#2026-07-18--ci-build-once-was-measured-and-rejected-keep-the-420-tree-skip) --- @@ -1884,3 +1885,23 @@ the three still-**silent** build-time dispatch sites — the ones this issue nam `SeasonEpisodeMediaCollectionEnumerator`) that simply aren't reverse-mapped, so making it "loud" would emit false-positive warnings. Completing that reverse map is a separate concern from "an unsupported *order* degrades silently" and is not part of #403's scope. +## 2026-07-18 — CI build-once was measured and rejected; keep the #420 tree-skip + +Build-once (a `compile` job producing a single artifact, consumed by `test`/`migrations`/ +`functional-e2e` via `--no-build`) was fully implemented and went **green on CI** (PR #455, run +830), then **rejected on measurement**: it traded a ~12% slot-occupancy saving for a ~40–85% +**per-run wall-clock regression**. + +- **Why it regressed.** The `bin`+`obj` artifact is 2.5 GB raw / 972 MB gz; tar alone costs ~82s CPU + plus ~180s transport, consuming most of the ~465s the shared compile was meant to save. Worse, + `compile` serializes **before** `migrations`' long ef-replay, which is pure DB work a shared build + cannot shorten — the bottleneck was never the redundant compiles. +- **Incidental finding worth recording.** `actions/upload-artifact@v4` does not work on this Gitea + instance — it throws `GHESNotSupportedError`, because the `@actions/artifact` v2 client library + rejects any non-`github.com` host. `@v3` is required for any future artifact use here. +- **Kept: the #420 cross-run tree-identity skip** (`docs/ci-cd.md` → Cross-run tree-identity skip). + It is independent of build-once — it only *skips* redundant work on identical-tree main pushes, at + zero wall-clock cost, rather than trying to share a build across jobs. Don't re-attempt build-once + unless the runner's artifact storage or network changes materially. + +Refs: #398 (closed), #420, PR #455. diff --git a/scripts/ci-detect-already-validated.sh b/scripts/ci-detect-already-validated.sh new file mode 100755 index 000000000..470353bfe --- /dev/null +++ b/scripts/ci-detect-already-validated.sh @@ -0,0 +1,90 @@ +#!/usr/bin/env bash +# scripts/ci-detect-already-validated.sh — emit `skip=true|false` to $GITHUB_OUTPUT for the +# docker-build.yml cross-run tree-identity gate (ersatztv#420). On a merge-to-main push, if the +# merged tree is byte-identical to a PR head that ALREADY passed `test`+`migrations` (a GREEN +# combined commit status), the heavy compile/test/migrations work is redundant -- the exact same +# source was already validated. `build` still runs and still builds+pushes the image, so no image +# ever ships from unvalidated source. +# +# The bias is ALWAYS toward running MORE, never less: a false 'true' would ship (or claim to +# validate) unreviewed/unvalidated source, so every ambiguous or unverifiable case resolves to +# skip=false. It is fine (just wasteful) to re-run on an identical tree; it is a correctness bug +# to skip validation on a tree that differs or was never proven green. +# +# Runs identically locally and in CI. Locally (no $GITHUB_OUTPUT) it prints the decision to +# stdout; e.g. GITHUB_EVENT_NAME=push GITHUB_REF=refs/heads/main scripts/ci-detect-already-validated.sh +set -euo pipefail + +out="${GITHUB_OUTPUT:-/dev/stdout}" +event="${GITHUB_EVENT_NAME:-}" +ref="${GITHUB_REF:-}" + +emit() { + echo "skip=$1" >> "$out" + echo "-> skip=$1" +} + +# Only a push directly to main can possibly be a merge-to-main we can cross-check against an +# already-validated PR head. Everything else (pull_request, tag push, workflow_dispatch, a push +# to any other branch) -> always run. +if [ "$event" != "push" ] || [ "$ref" != "refs/heads/main" ]; then + echo "event='${event:-}' ref='${ref:-}' (need push to refs/heads/main); running full validation (safe default)" + emit false + exit 0 +fi + +# Deepen history so HEAD's second parent (and its tree) are resolvable -- a shallow checkout may +# have neither. Mirrors the docs-only/docs-reminder jobs' fetch style. Non-fatal: a failed +# deepen still falls through to the HEAD^2 check below, which then fails safe. +git fetch --deepen=2 origin 2>/dev/null || git fetch --unshallow origin 2>/dev/null || true + +# HEAD must be a real merge commit with a second parent -- that second parent is the PR head CI +# actually validated. No second parent (a direct/fast-forward/squash push) -> nothing to compare +# against -> always run. +pr_head="$(git rev-parse --verify -q HEAD^2 || true)" +if [ -z "$pr_head" ]; then + echo "HEAD has no second parent (not a merge commit); running full validation (safe default)" + emit false + exit 0 +fi + +merge_tree="$(git rev-parse --verify -q 'HEAD^{tree}' || true)" +pr_tree="$(git rev-parse --verify -q "${pr_head}^{tree}" || true)" +if [ -z "$merge_tree" ] || [ -z "$pr_tree" ]; then + echo "could not resolve HEAD or HEAD^2 tree; running full validation (safe default)" + emit false + exit 0 +fi + +if [ "$merge_tree" != "$pr_tree" ]; then + echo "merged tree ($merge_tree) != PR head $pr_head tree ($pr_tree) -- main advanced since the PR was validated; running full validation" + emit false + exit 0 +fi + +# Merge tree matches the PR head tree exactly. Confirm that PR head was actually validated green +# before trusting it -- query the Gitea combined commit status API. Auth is required (private +# instance); a missing/failing/non-success response always falls through to skip=false. +if [ -z "${ETV_STATUS_AUTH:-}" ]; then + echo "ETV_STATUS_AUTH not set; cannot verify PR head status; running full validation (safe default)" + emit false + exit 0 +fi + +status_url="http://192.168.1.95:3000/api/v1/repos/timothy/ersatztv/commits/${pr_head}/status" +status_json="$(curl -sf -u "$ETV_STATUS_AUTH" "$status_url" || true)" +if [ -z "$status_json" ]; then + echo "status API request for PR head ${pr_head} failed; running full validation (safe default)" + emit false + exit 0 +fi + +state="$(printf '%s' "$status_json" | jq -r '.state // empty' 2>/dev/null || true)" +if [ "$state" != "success" ]; then + echo "PR head ${pr_head} combined status is '${state:-}', not 'success'; running full validation (safe default)" + emit false + exit 0 +fi + +echo "merged tree == green PR head ${pr_head} (status=success) -> skipping re-validation" +emit true