process: #311 H11 rebase-hook + PR-scoped format CI + #312 H12 qualification audit #313

Merged
timothy merged 3 commits from ci/311-rebase-hook-format-ci into main 2026-07-12 20:32:19 +02:00
7 changed files with 222 additions and 7 deletions
+32
View File
@@ -0,0 +1,32 @@
#!/usr/bin/env bash
# H11 (ersatztv#311) — refuse to push a branch that is BEHIND origin/main: rebase first, do NOT
# merge main in. A merge commit drags in files you never touched (e.g. the ~2500 legacy-BOM .cs),
# which then trips the pre-commit `dotnet format` hook on code that isn't yours (the #309 session).
# Rebasing keeps your diff to exactly what you changed.
#
# Fail-OPEN on anything we can't decide (a git pre-push hook has no "ask"): not a git repo,
# offline / fetch fails, no origin/main, HEAD unresolved -> allow the push. The only hard block is
# a positively-proven "behind origin/main". Deliberate exception: ETV_SKIP_REBASE_CHECK=1.
set -uo pipefail
[ "${ETV_SKIP_REBASE_CHECK:-}" = "1" ] && exit 0
git rev-parse --git-dir >/dev/null 2>&1 || exit 0
# Best-effort fetch of the latest main; offline / no network -> don't block.
git fetch origin main --quiet 2>/dev/null || exit 0
git rev-parse --verify --quiet origin/main >/dev/null 2>&1 || exit 0
# Pushing main itself, or a branch already rebased on top of it, means origin/main is an ANCESTOR
# of HEAD -> nothing to rebase, allow.
if git merge-base --is-ancestor origin/main HEAD 2>/dev/null; then
exit 0
fi
behind=$(git rev-list --count HEAD..origin/main 2>/dev/null || echo '?')
branch=$(git rev-parse --abbrev-ref HEAD 2>/dev/null || echo HEAD)
echo "husky - push blocked (H11): '$branch' is behind origin/main by $behind commit(s)."
echo " Rebase before pushing — do NOT merge main in (a merge drags in files you didn't touch,"
echo " e.g. legacy-BOM .cs, and trips the format hook on code that isn't yours):"
echo " git fetch origin main && git rebase origin/main"
echo " Deliberate exception: ETV_SKIP_REBASE_CHECK=1 git push"
exit 1
+61
View File
@@ -478,3 +478,64 @@ jobs:
exit 1
fi
echo "Generated API artifacts are in sync."
# Formatting-as-you-touch gate (ersatztv#311): verify the .cs files THIS PR changed conform to
# .editorconfig (style + charset=utf-8, i.e. no UTF-8 BOM). Scoped to changed files so it enforces
# "normalize a legacy file when you touch it" WITHOUT a big-bang reformat of the ~2500 pre-existing
# BOM files. A PR that touches no .cs skips the expensive steps and passes trivially (always reports
# a status, so it is safe as a required check).
format:
name: Formatting (changed .cs conform to .editorconfig)
runs-on: ubuntu-latest
if: github.event_name == 'pull_request'
steps:
- name: Checkout
uses: actions/checkout@v4
with:
fetch-depth: 0
- name: Detect changed C# files
id: detect
run: |
base_ref="${{ github.base_ref }}"
git fetch --no-tags --depth=100 origin "$base_ref" || true
changed="$(git diff --name-only --diff-filter=ACM "origin/${base_ref}...HEAD" -- '*.cs' 2>/dev/null || true)"
echo "Changed .cs files in this PR:"; printf '%s\n' "$changed"
if [ -n "$changed" ]; then
printf '%s\n' "$changed" > /tmp/changed-cs.txt
echo "cs_changed=true" >> "$GITHUB_OUTPUT"
echo "-> will verify these files conform to .editorconfig."
else
echo "cs_changed=false" >> "$GITHUB_OUTPUT"
echo "No .cs change -> skipping format verify (job passes)."
fi
- name: Setup .NET
if: steps.detect.outputs.cs_changed == 'true'
uses: actions/setup-dotnet@v4
with:
dotnet-version: '10.0.x'
- name: Cache NuGet packages
if: steps.detect.outputs.cs_changed == 'true'
uses: actions/cache@v4
with:
path: ~/.nuget/packages
key: nuget-${{ runner.os }}-${{ hashFiles('Directory.Packages.props', 'global.json') }}
restore-keys: nuget-${{ runner.os }}-
- name: Restore
if: steps.detect.outputs.cs_changed == 'true'
run: dotnet restore
- name: Verify formatting of changed .cs files
if: steps.detect.outputs.cs_changed == 'true'
shell: bash
run: |
mapfile -t files < /tmp/changed-cs.txt
echo "Verifying ${#files[@]} changed .cs file(s) against .editorconfig..."
if ! dotnet format ErsatzTV.sln --no-restore --verify-no-changes --include "${files[@]}"; then
echo "::error::One or more .cs files this PR touches don't conform to .editorconfig (formatting or a UTF-8 BOM). Run 'dotnet format ErsatzTV.sln --include <files>' and commit the result in THIS PR — the fix-as-you-touch convention (docs/contributing.md §7; ersatztv#311). Legacy files you did NOT touch are unaffected."
exit 1
fi
echo "All changed .cs files conform to .editorconfig."
+5
View File
@@ -10,6 +10,11 @@ printf '%s\n' "$_prepush_refs" | ./.claude/hooks/prepush-donewhen.sh || exit 1
# diff" and lets drift through. Unset them so nested git rediscovers the repo normally.
unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE
# H11 (ersatztv#311): refuse to push a branch that is BEHIND origin/main — rebase, don't merge
# main in (a merge drags in files you never touched, e.g. legacy-BOM .cs, and trips the format
# hook on code that isn't yours). Fail-open; escape with ETV_SKIP_REBASE_CHECK=1.
./.claude/hooks/prepush-rebase-check.sh || exit 1
# CI-parity checks: catch "green locally, red in CI" before the push leaves the machine.
# check:api guards the generated OpenAPI types (v1.json / v1.d.ts drift); the full
# lint/typecheck/build catch a staged change that breaks an UNstaged file (lint-staged
+9
View File
@@ -99,6 +99,15 @@ talks exclusively to the REST API. Every former Blazor route now 302-redirects t
- **`.editorconfig` is the source of truth** for formatting + rule severities (plus
`ErsatzTV.sln.DotSettings` for ReSharper). Run `dotnet format` before committing.
- **Fix formatting as you touch it (no big-bang).** ~2500 legacy `.cs` files inherited from upstream
carry a UTF-8 BOM, which violates `.editorconfig`'s `charset=utf-8`. We do **not** mass-reformat.
Instead, **when you modify a file for other work, normalize it in that same PR**`dotnet format
ErsatzTV.sln --include <the files you touched>`, which strips the BOM and fixes style. Files you did
not touch stay as-is. This is enforced two ways so it can't be silently skipped (ersatztv#311): the
pre-commit hook verifies staged `.cs`, and a blocking **`format`** CI job re-verifies the `.cs` this
PR changed against `.editorconfig` (a `.cs`-free PR passes trivially). Never `--no-verify` past a
format failure on a file you changed — de-BOM/format it instead. (A one-time repo-wide normalization
is a separate, unmade decision; the touched-file rule is the standing one.)
- **`TreatWarningsAsErrors=true`** in the app projects — a warning fails the build. `NoWarn` carries a
small, documented exemption list (e.g. `VSTHRD200`, `CA1873`); NuGet-audit `NU1901-1903` are demoted
to warnings in `Directory.Build.props` while `NU1904` (critical) blocks.
+34
View File
@@ -77,6 +77,7 @@ keep append-only from accreting stale, contradictory, or unreadably-large histor
- [2026-07-12 — Review-verdict merge-gate: latest commit must be reviewed (#303 H10)](#2026-07-12--review-verdict-merge-gate-latest-commit-must-be-reviewed-303-h10)
- [2026-07-12 — Cross-editor ETag rotation completed for Collection/Playout config siblings (#269)](#2026-07-12--cross-editor-etag-rotation-completed-for-collectionplayout-config-siblings-269)
- [2026-07-12 — Live-E2E is a required step for API write-path handler changes (#303)](#2026-07-12--live-e2e-is-a-required-step-for-api-write-path-handler-changes-303)
- [2026-07-12 — Formatting-as-you-touch, enforced; rebase-not-merge for PR branches (#311 H11 + format CI)](#2026-07-12--formatting-as-you-touch-enforced-rebase-not-merge-for-pr-branches-311-h11--format-ci)
---
@@ -1486,3 +1487,36 @@ silent: the PR/close comment states that live-E2E ran, or — for a non-write-pa
wasn't required (the same stated-exemption discipline as the review skip rubric). Recipe +
"When live-E2E is required": `docs/e2e-local.md`. This formalizes the #229 lore bullet ("live E2E
remains the only net for this class") into a standing convention.
---
## 2026-07-12 — Formatting-as-you-touch, enforced; rebase-not-merge for PR branches (#311 H11 + format CI)
Two coupled process decisions, prompted when a stale docs branch *merged main in*, dragged ~17
legacy-BOM `.cs` files it never touched into the merge commit, and the pre-commit `dotnet format`
hook then blocked on code that wasn't the author's (#309 session; the BOM backlog is #310).
**1. Formatting-as-you-touch is the standing rule, and it is now *enforced* (not just prose).** ~2500
of ~3900 `.cs` files carry a legacy UTF-8 BOM that violates `.editorconfig`'s `charset=utf-8`. We do
**not** mass-reformat (a repo-wide normalization stays an unmade, separate decision). Instead, a file
you modify for other work must be normalized (`dotnet format`, incl. BOM strip) in that same PR.
Enforcement — previously only the `--no-verify`-bypassable pre-commit hook, which is how #269 landed 17
BOM files (CI never checked charset):
- a **blocking `format` CI job** runs `dotnet format --verify-no-changes` **scoped to the PR's changed
`.cs`** (vs the merge-base) — so it demands conformance only of files the PR touched, never the
untouched legacy 2500; a `.cs`-free PR skips the expensive steps and passes (always reports a status,
safe as a required check). This closes the "CI never verifies charset/format" gap.
- `docs/contributing.md` §7 documents the rule.
**2. Keep a PR branch current by REBASING on `origin/main`, never merging main in (H11).** A merge
commit pulls in *every* file main changed — including files the author never touched — which then trip
the format hook/CI on code that isn't theirs; rebasing keeps the diff to exactly what changed.
Enforced by `.claude/hooks/prepush-rebase-check.sh` wired into `.husky/pre-push`: a push from a branch
that is behind `origin/main` (origin/main not an ancestor of HEAD) is **blocked** with
`git rebase origin/main` guidance. Fail-open (offline / no origin/main / not a repo → allow, since a
git hook has no "ask"); deliberate escape `ETV_SKIP_REBASE_CHECK=1`. This supersedes the old lore
guidance to "merge main into your PR branch." (After a rebase that conflicts in *generated* artifacts —
v1.json/v1.d.ts/endpoint-index — regenerate, don't hand-resolve; `npm run check:api` guards.)
Rationale, as with the whole hook program: make the process rule a derivation/hook, not prose to
remember (#303 methodology review). Tracked: #311; sibling #312 (H12 issue-qualification audit).
+21 -7
View File
@@ -62,9 +62,11 @@ Then work the queue:
reviewer-repo audits are claimed by comment only.
4. Read the issue bodies (they carry the task context/evidence) and work the item under the
HARD CONSTRAINTS below.
5. Finish by following the session-end protocol in #237: ONE session comment on the tracker
(template in the tracker body, incl. triage verdicts for any new issues), remove your
`in-progress` labels, and complete the per-issue Task Completion Protocol from CLAUDE.md.
5. Finish by following the session-end protocol in #237: run the **H12 qualification audit**
(`ETV_GITEA_BASICAUTH=user:pass scripts/issue-qualification-audit.sh`) and add a `priority:`
label to anything it lists (every issue you filed this session included); then ONE session
comment on the tracker (template in the tracker body, incl. triage verdicts for any new issues),
remove your `in-progress` labels, and complete the per-issue Task Completion Protocol from CLAUDE.md.
HARD CONSTRAINTS:
- Work in worktrees off origin/main. Copy web/node_modules from the main checkout.
@@ -153,10 +155,22 @@ HARD CONSTRAINTS:
survived a green fixed-point test because the handler returned a lazy `Map` the test never enumerated.
Enumerate handler return values in tests; live E2E remains the only net for this class. (Write-path
reload-through-read-path mechanics: api-conventions §7.)
- **Merging main into an open PR branch: REGENERATE generated artifacts** (v1.json, v1.d.ts,
endpoint-index) after resolving source conflicts — git text-merges them plausibly-but-wrong;
`npm run check:api` is the guard. Also expect other sessions to push to YOUR PR branch
(merge-main updates): `git pull` before push, treat non-FF as signal not error.
- **Keep a PR branch current by REBASING on origin/main — do NOT merge main in** (ersatztv#311,
enforced by the H11 pre-push hook: a push from a branch behind origin/main is blocked → `git fetch
origin main && git rebase origin/main`). A *merge* commit pulls in every file main changed, incl.
files you never touched (e.g. the ~2500 legacy-BOM `.cs`), which then trip the format hook/CI on code
that isn't yours (the #309 session). Rebasing keeps your diff to exactly what you changed. After a
rebase that hits conflicts in **generated artifacts** (v1.json, v1.d.ts, endpoint-index), REGENERATE
them (`./scripts/update-openapi.sh` + `npm run generate:api`) — never hand-resolve; git text-merges
them plausibly-but-wrong and `npm run check:api` is the guard. Escape hatch for a deliberate
non-rebased push: `ETV_SKIP_REBASE_CHECK=1 git push`.
- **H12 issue-qualification audit** (ersatztv#312, `scripts/issue-qualification-audit.sh`): a
session-end check that lists OPEN issues missing a `priority:` label — the #237 ranking keys off
`priority:`/gate labels, so an unlabeled issue is invisible to it. "Fully qualified" = has a
`priority: {high,medium,low}` label (that signals triage ran; gate-vs-backlog is then derivable
from the `review` label / milestone, and a milestone is NOT required — backlog is unmilestoned).
Run it at session end and label anything it flags (esp. issues you filed this session). Fail-open
without creds; advisory (exit 1 when any are unqualified). Sibling to H11 (both #311/#312).
- **CI VM test timeouts**: heavy-render web tests (100+ item grids) need explicit vitest
timeouts (e.g. 15s) — the CI VM hit the 5s default on a test that runs in ~1s locally
(run 686). Bump per-test, don't raise the global default.
+60
View File
@@ -0,0 +1,60 @@
#!/usr/bin/env bash
# H12 (ersatztv#312) — session-end audit: list OPEN issues that are not "fully qualified" for the
# #237 ranking protocol. An issue is qualified when it carries a `priority:` label — that is the
# signal triage happened; gate-vs-backlog is then derivable (a `review` label / open milestone =
# gate, otherwise backlog), and a milestone is deliberately NOT required (backlog is unmilestoned).
# So the one mandatory check is: does the issue have a `priority: {high,medium,low}` label?
#
# Run it at session end (kickoff session-end protocol) and qualify anything it lists before closing.
# Advisory: prints the under-qualified set and exits 1 if any exist, 0 if all clean. Fail-OPEN with
# no creds / Gitea unreachable (prints a notice, exits 0 — never a spurious signal).
#
# Creds from env: ETV_GITEA_TOKEN or ETV_GITEA_BASICAUTH (user:pass). ETV_GITEA_URL overrides the
# base (default: the LAN instance); ETV_GITEA_REPO overrides owner/repo (default timothy/ersatztv).
set -uo pipefail
base_url="${ETV_GITEA_URL:-http://192.168.1.95:3000}/api/v1"
repo="${ETV_GITEA_REPO:-timothy/ersatztv}"
gq() {
local path="$1"
if [ -n "${ETV_GITEA_TOKEN:-}" ]; then
curl -sf -H "Authorization: token $ETV_GITEA_TOKEN" "$base_url/$path" 2>/dev/null
elif [ -n "${ETV_GITEA_BASICAUTH:-}" ]; then
curl -sf -u "$ETV_GITEA_BASICAUTH" "$base_url/$path" 2>/dev/null
else
return 1
fi
}
if [ -z "${ETV_GITEA_TOKEN:-}" ] && [ -z "${ETV_GITEA_BASICAUTH:-}" ]; then
echo "H12 audit: no Gitea creds in env (ETV_GITEA_TOKEN or ETV_GITEA_BASICAUTH) — skipping (no-op)."
exit 0
fi
# Page through OPEN issues (type=issues excludes PRs; .pull_request guard is belt-and-suspenders).
# Emit "#N<TAB>title" for any issue with NO 'priority:' label.
missing=""
page=1
while :; do
batch=$(gq "repos/$repo/issues?state=open&type=issues&limit=50&page=$page") || {
echo "H12 audit: Gitea unreachable — skipping (no-op)."; exit 0; }
count=$(printf '%s' "$batch" | jq 'length' 2>/dev/null || echo 0)
[ "${count:-0}" -eq 0 ] && break
rows=$(printf '%s' "$batch" | jq -r '
.[]
| select(.pull_request == null)
| select(([ (.labels // [])[].name | select(startswith("priority:")) ] | length) == 0)
| "#\(.number)\t\(.title)"' 2>/dev/null || true)
[ -n "$rows" ] && missing="${missing}${rows}"$'\n'
page=$((page + 1))
done
if [ -z "$(printf '%s' "$missing" | tr -d '[:space:]')" ]; then
echo "H12 audit: every open issue carries a priority: label. ✓"
exit 0
fi
echo "H12 audit: OPEN issues missing a 'priority:' label — qualify these before session end"
echo "(the #237 ranking keys off priority:/gate labels; an unlabeled issue is invisible to it):"
printf '%s' "$missing" | sed '/^[[:space:]]*$/d' | sed 's/^/ /'
exit 1