From 74005cc952718069f28742febe5f1c6853d99a30 Mon Sep 17 00:00:00 2001 From: Timothy Date: Fri, 17 Jul 2026 21:33:53 +0200 Subject: [PATCH 1/2] fix(416): detect against FETCH_HEAD with a two-dot diff (shallow-checkout safe) The test/migrations jobs check out fetch-depth:1. A shallow clone has NO origin/ tracking ref and no merge-base, so the three-dot origin/main...HEAD errored -> empty diff -> docs_only=false -> EVERY docs-only PR silently ran the full matrix (safe but the skip never fired). git fetch always writes FETCH_HEAD, which resolves in a shallow clone; diff against it with a two-dot tree diff (no merge-base). Confirmed in a real shallow file:// clone: origin/main did NOT resolve and three-dot errored, while FETCH_HEAD two-dot correctly returned the docs file. api-docs/format were unaffected only because they use fetch-depth:0. Refs #416 --- scripts/ci-detect-docs-only.sh | 55 +++++++++++++++++++++++----------- 1 file changed, 37 insertions(+), 18 deletions(-) diff --git a/scripts/ci-detect-docs-only.sh b/scripts/ci-detect-docs-only.sh index 2ca1072ea..edc5a00aa 100755 --- a/scripts/ci-detect-docs-only.sh +++ b/scripts/ci-detect-docs-only.sh @@ -37,7 +37,15 @@ if [ "${GITHUB_REF_TYPE:-}" = "tag" ]; then exit 0 fi -range="" +# Resolve a concrete BASE COMMIT to diff HEAD against. SHALLOW-CHECKOUT SAFE (ersatztv#416 +# follow-up): the `test`/`migrations` jobs check out `fetch-depth: 1`, and a shallow clone has NO +# `origin/` remote-tracking ref and no merge-base — so the old `origin/...HEAD` +# (three-dot) errored, the diff came back empty, and EVERY docs-only PR silently ran the full +# matrix (safe but useless). `git fetch` ALWAYS writes FETCH_HEAD (the fetched base tip), which +# resolves even in a shallow clone; we diff against it with a TWO-dot tree diff below (no merge-base +# required). Verified in a real shallow `file://` clone. (`api-docs`/`format` only worked because +# they use `fetch-depth: 0`.) +base_rev="" case "$event" in pull_request) base="${GITHUB_BASE_REF:-}" @@ -46,19 +54,23 @@ case "$event" in emit false exit 0 fi - git fetch --no-tags --depth=200 origin "$base" || true - range="origin/${base}...HEAD" + if ! git fetch --no-tags --depth=200 origin "$base" 2>/dev/null; then + echo "fetch of base '$base' failed; running full matrix (safe default)" + emit false + exit 0 + fi + base_rev="$(git rev-parse --verify -q FETCH_HEAD || true)" ;; push) - # Only branch pushes reach here (tags handled above). We skip the heavy matrix on a docs-only - # push only when the range is UNAMBIGUOUS — i.e. HEAD is a merge commit, which is how every - # update to main lands (PR merge). Its first parent is the pre-merge branch tip, so - # HEAD^1...HEAD is exactly the merged delta. A non-merge (direct/multi-commit) push is rare and - # its true range is ambiguous here, so we fall back to running the full matrix. - git fetch --no-tags --depth=200 origin "${GITHUB_REF_NAME:-main}" || true - nfields="$(git rev-list --parents -n1 HEAD | wc -w | tr -d ' ')" # 1 (self) + parent count + # Only branch pushes reach here (tags handled above). Skip only when the range is UNAMBIGUOUS — + # HEAD is a merge commit (how every update to main lands): its first parent is the pre-merge + # tip, so HEAD^1 is the base. Fetch depth 2 so the first-parent tree exists in the shallow + # checkout; if it still can't be resolved/diffed we fall through to the full matrix. A non-merge + # (direct/multi-commit) push is ambiguous here -> full matrix. + git fetch --no-tags --depth=2 origin "${GITHUB_REF_NAME:-main}" 2>/dev/null || true + nfields="$(git rev-list --parents -n1 HEAD 2>/dev/null | wc -w | tr -d ' ')" # 1 (self) + parents if [ "${nfields:-0}" -ge 3 ]; then - range="HEAD^1...HEAD" + base_rev="$(git rev-parse --verify -q 'HEAD^1' || true)" else echo "non-merge push (parents=$(( nfields - 1 ))); running full matrix (safe default)" emit false @@ -72,13 +84,20 @@ case "$event" in ;; esac -# --no-renames is load-bearing: with rename detection ON (git's default) a code->docs rename -# (e.g. Foo.cs -> docs/Foo.md) shows ONLY the destination `docs/Foo.md`, hiding that a source file -# left the build -> misclassified as docs-only -> required tests skipped on a code change. -# --no-renames surfaces the deletion (`Foo.cs`, non-docs) so it correctly forces the full matrix, -# keeping the "any code path => run everything" invariant total. -changed="$(git diff --no-renames --name-only "$range" 2>/dev/null || true)" -echo "Range: $range" +if [ -z "$base_rev" ]; then + echo "could not resolve a base revision (shallow/offline); running full matrix (safe default)" + emit false + exit 0 +fi + +# TWO-dot tree diff `git diff HEAD` (NOT three-dot) so no merge-base is needed — that is what +# makes this work in a shallow checkout. `--no-renames` is load-bearing: with rename detection ON +# (git's default) a code->docs rename (e.g. Foo.cs -> docs/Foo.md) shows ONLY the destination +# `docs/Foo.md`, hiding that a source file left the build -> misclassified as docs-only -> required +# tests skipped on a code change. --no-renames surfaces the deletion (`Foo.cs`, non-docs) so it +# correctly forces the full matrix, keeping the "any code path => run everything" invariant total. +changed="$(git diff --no-renames --name-only "$base_rev" HEAD 2>/dev/null || true)" +echo "Base: $base_rev (event=$event)" echo "Changed files:" printf '%s\n' "$changed" From 082124f706d920d1ab465ad702e0c147bc1214ce Mon Sep 17 00:00:00 2001 From: Timothy Date: Fri, 17 Jul 2026 22:02:06 +0200 Subject: [PATCH 2/2] docs(416): note shallow-checkout FETCH_HEAD/two-dot fix (ci-cd + decisions) Refs #416 --- docs/ci-cd.md | 9 +++++++++ docs/decisions.md | 15 +++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/docs/ci-cd.md b/docs/ci-cd.md index 711ded711..c62b3003c 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -316,6 +316,15 @@ files) has nothing for the heavy jobs to validate. Before this, such a change ra is gated `if: steps.detect.outputs.docs_only != 'true'`. On a docs-only change the job runs only checkout + detect and **reports `success` in seconds**. +**Shallow-checkout safe (the change-set diff).** `test`/`migrations` check out `fetch-depth: 1`, and +a shallow clone has **no `origin/` tracking ref and no merge-base** — so a three-dot +`origin/main...HEAD` diff *errors*, the fail-safe returns `docs_only=false`, and the skip silently +never fires (the first cut shipped this bug — every docs-only PR still ran the full matrix; caught by +#416's "verify on a real PR" box). The script therefore fetches the base and diffs against +**`FETCH_HEAD`** (always written by `git fetch`, resolves in a shallow clone) with a **two-dot** tree +diff (`git diff --no-renames FETCH_HEAD HEAD`) — no merge-base required. (`api-docs`/`format` avoided +the bug only because they check out `fetch-depth: 0`.) + The jobs are **not** `if:`-skipped. That is deliberate and it is the whole trap of this issue: - `main`'s branch protection requires two checks **by name** — `Build ErsatzTV Image / Build & test diff --git a/docs/decisions.md b/docs/decisions.md index 105870b54..b05454556 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -1522,3 +1522,18 @@ part — is skipped. Two adjacent redundancies are deliberately **out of scope**: the whole matrix re-running on a PR and again on the merge-to-`main` over identical code (#420), and the within-run triple `dotnet build` (#398). Full mechanism in `ci-cd.md` → "Docs-only skip". +## 2026-07-17 — Docs-only detect must be shallow-checkout safe: FETCH_HEAD + two-dot, not origin/main + three-dot (#416 follow-up) + +The #416 docs-only skip shipped (#422) safe but **ineffective**: every docs-only PR still ran the full +matrix. Root cause — `test`/`migrations` check out `fetch-depth: 1`, and in a shallow clone +`origin/` has no remote-tracking ref and there is no merge-base, so the detect script's +three-dot `git diff origin/main...HEAD` errored → `|| true` → empty diff → the fail-safe returned +`docs_only=false` → full matrix. Confirmed in a real shallow `file://` clone (`origin/main` did not +resolve; three-dot errored; `git diff FETCH_HEAD HEAD` returned the changed files correctly). + +Decision: the detect diffs against **`FETCH_HEAD`** (always written by `git fetch`, resolves in a +shallow clone) with a **two-dot** tree diff (no merge-base). `api-docs`/`format` were unaffected only +because they use `fetch-depth: 0` — a difference the first cut missed. Meta-lesson reinforced: a CI +gating change can pass every local test and merge green while being a complete no-op in CI; only +real-PR verification that **measures the effect** (job durations, not just a green check) catches it — +which is exactly what #416's Done-when demanded. Fixed in the #416 follow-up PR.