chore: PreToolUse guard that blocks a commit/push with a BOM'd touched .cs #413

Merged

timothy merged 2 commits from chore/bom-guard-hook into main 2026-07-17 17:50:02 +02:00

Conversation 1 Commits 2 Files Changed 3

+121 -4

timothy commented 2026-07-17 16:17:08 +02:00

Owner

Copy Link

Copy Source

Adds a third enforcement layer to the #311 fix-as-you-touch formatting rule: a Claude PreToolUse/Bash guard that denies git commit / git push when a .cs file the branch touches still carries a UTF-8 BOM.

Why, concretely

The rule is already enforced by a pre-commit hook and a blocking CI job — and both missed the same mistake twice on 2026-07-17: PR #405 shipped 6 BOM'd files, PR #402 shipped 19.

The gap: the pre-commit hook is routinely skipped. Worktree hook friction makes git commit --no-verify the normal path in this repo, which bypasses it — leaving CI, a ~10-minute round trip, as the first thing that tells you. This guard fires on Bash before git runs, so --no-verify can't skip it.

Why a hook and not a note: a memory describing this exact trap did not prevent either failure. The PR #402 session re-added a BOM an hour after writing that memory down — because the usual ways of touching a legacy file re-add it silently: Python io.open(..., encoding='utf-8-sig') writes a BOM back, and sed/perl round-trips preserve it. A check that runs beats one you have to remember.

Design

Follows the existing gate pattern — a fail-open script in .claude/hooks/ registered on the PreToolUse/Bash matcher beside bash-guard and worktree-guard.

• Fail-open: any parse/lookup trouble → allow. This must never be the reason a commit can't happen; CI stays the backstop.
• Generated files exempt (*.Designer.cs, TvContextModelSnapshot.cs) — matching what dotnet format and the CI verify already skip, so dotnet ef output doesn't trip it.
• Scoped to this repo.
• Resolves the target tree from an in-command cd, because commits here run as cd <worktree> && git ... and the harness resets the shell cwd between calls (falls back to payload cwd, then project dir).
• On a hit it names the files and gives the strip command.

Verified against a throwaway fixture

Case	Result
git commit with a BOM'd touched file	deny (names the file + fix)
git push with a BOM	deny
BOM only in generated *.Designer.cs / snapshot	allow
Clean tree	allow
git status with a BOM present	allow (not a gated verb)
echo 'run git push later'	allow (no false-trip on the words)

Also proved live via a temporary sentinel — it fires on the Bash matcher — sentinel removed, jq -e schema check passes, and the two existing Bash hooks are preserved (appended, not replaced).

Docs

docs/contributing.md — three enforcement layers and why the third exists; the utf-8-sig re-add trap; a by-hand check; and a caveat worth its own line: dotnet format --include needs bash. mapfile is bash-only, so under zsh the file list comes out empty and the tool looks like it silently did nothing — which is how it acquired a reputation for no-op'ing on this machine. It doesn't; it reproduces CI exactly.

No production code touched — .claude/ tooling + one docs bullet.

🤖 Generated with Claude Code

Adds a third enforcement layer to the #311 fix-as-you-touch formatting rule: a Claude `PreToolUse`/`Bash` guard that **denies `git commit` / `git push`** when a `.cs` file the branch touches still carries a UTF-8 BOM. ## Why, concretely The rule is already enforced by a pre-commit hook *and* a blocking CI job — and **both missed the same mistake twice on 2026-07-17**: PR #405 shipped 6 BOM'd files, PR #402 shipped 19. The gap: **the pre-commit hook is routinely skipped.** Worktree hook friction makes `git commit --no-verify` the normal path in this repo, which bypasses it — leaving CI, a ~10-minute round trip, as the first thing that tells you. This guard fires on Bash *before* git runs, so `--no-verify` can't skip it. **Why a hook and not a note:** a memory describing this exact trap did not prevent either failure. The PR #402 session re-added a BOM **an hour after writing that memory down** — because the usual ways of touching a legacy file re-add it silently: Python `io.open(..., encoding='utf-8-sig')` **writes a BOM back**, and sed/perl round-trips preserve it. A check that runs beats one you have to remember. ## Design Follows the existing gate pattern — a fail-open script in `.claude/hooks/` registered on the `PreToolUse`/`Bash` matcher beside `bash-guard` and `worktree-guard`. - **Fail-open**: any parse/lookup trouble → allow. This must never be the reason a commit can't happen; CI stays the backstop. - **Generated files exempt** (`*.Designer.cs`, `TvContextModelSnapshot.cs`) — matching what `dotnet format` and the CI verify already skip, so `dotnet ef` output doesn't trip it. - **Scoped to this repo.** - **Resolves the target tree from an in-command `cd`**, because commits here run as `cd <worktree> && git ...` and the harness resets the shell cwd between calls (falls back to payload cwd, then project dir). - On a hit it **names the files and gives the strip command**. ## Verified against a throwaway fixture | Case | Result | |---|---| | `git commit` with a BOM'd touched file | **deny** (names the file + fix) | | `git push` with a BOM | **deny** | | BOM only in generated `*.Designer.cs` / snapshot | allow | | Clean tree | allow | | `git status` with a BOM present | allow (not a gated verb) | | `echo 'run git push later'` | allow (no false-trip on the words) | Also **proved live** via a temporary sentinel — it fires on the Bash matcher — sentinel removed, `jq -e` schema check passes, and the two existing Bash hooks are preserved (appended, not replaced). ## Docs `docs/contributing.md` — three enforcement layers and why the third exists; the `utf-8-sig` re-add trap; a by-hand check; and a caveat worth its own line: **`dotnet format --include` needs bash**. `mapfile` is bash-only, so under zsh the file list comes out empty and the tool looks like it silently did nothing — which is how it acquired a reputation for no-op'ing on this machine. It doesn't; it reproduces CI exactly. No production code touched — `.claude/` tooling + one docs bullet. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

timothy added 1 commit 2026-07-17 16:17:08 +02:00

chore: PreToolUse guard that blocks a commit/push with a BOM'd touched .cs ... 9e1fce1a3a

The #311 fix-as-you-touch rule is already enforced by a pre-commit hook and a
blocking CI job, and both missed the same mistake twice on 2026-07-17: PR #405
shipped 6 BOM'd files, PR #402 shipped 19. The gap is that the pre-commit hook is
routinely skipped — worktree hook friction makes `git commit --no-verify` the
normal path here — which leaves CI, a ~10-minute round trip, as the first thing
that tells you.

This guard fires on Bash before git runs, so --no-verify can't skip it. It denies
`git commit`/`git push` when a .cs the branch touches still carries a BOM, names
the files, and gives the strip command.

Why a hook rather than a note: a memory describing this exact trap did not prevent
either failure — the PR #402 session re-added a BOM an hour after writing that
memory down, because the usual ways of touching a legacy file re-add it silently
(Python io.open(..., encoding='utf-8-sig') WRITES a BOM back; sed/perl round-trips
keep it). A check that runs beats one you have to remember.

Follows the existing gate pattern: a fail-open script in .claude/hooks/ registered
on the PreToolUse/Bash matcher alongside bash-guard and worktree-guard. Any parse
or lookup trouble → allow; this must never be the reason a commit can't happen, and
CI remains the backstop. Generated *.Designer.cs / TvContextModelSnapshot.cs are
exempt, matching what dotnet format itself skips. Scoped to this repo, and it
resolves the target tree from an in-command `cd` because commits here run as
`cd <worktree> && git ...` and the harness resets the shell cwd between calls.

Verified against a throwaway fixture: denies commit and push for a BOM'd touched
file (naming it); allows a clean tree, a BOM in generated files only, `git status`
with a BOM present, and `echo 'run git push later'` (no false-trip on the words).
Proved live via a sentinel — it fires on the Bash matcher — sentinel removed.

docs/contributing.md updated: three enforcement layers, why the third exists, the
utf-8-sig re-add trap, a by-hand check, and the bash-not-zsh caveat for
`dotnet format --include` (mapfile is bash-only; under zsh the file list is empty
and the tool looks like it silently did nothing).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

timothy referenced this pull request 2026-07-17 16:59:02 +02:00

feat(70): weighted / fair-share content distribution (WeightedShuffle playback order) #402

timothy added 1 commit 2026-07-17 17:03:04 +02:00

chore: widen the BOM-guard remedy to staged + dirty files (review nit) ... 030a7c19e9

Cold review (MERGEABLE-WITH-NITS) flagged that the printed fix snippet and the
docs by-hand check used `git diff origin/main...HEAD`, which excludes
staged-but-uncommitted files. A brand-new staged .cs is exactly what fires the
pre-commit deny yet is absent from that diff, so the copy-paste remedy would strip
nothing → deny again → loop. (The deny still names the file, so manual stripping
always worked; this just makes the printed remedy match the guard's own detection
set: branch diff ∪ staged ∪ dirty.)

Verified on a fresh fixture: a brand-new staged BOM file is denied, and the
widened snippet strips it.

Left as accepted (fail-open, Medium, CI backstops; shared with sibling hooks):
`git -C <path>` isn't parsed for the target tree (only `cd` is — matches the
`cd <wt> && git` usage here), and the heredoc-body false-deny vector that
bash-guard/worktree-guard also carry.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

timothy commented 2026-07-17 17:03:24 +02:00

Author

Owner

Copy Link

Copy Source

Cold review returned MERGEABLE-WITH-NITS @ 9e1fce1a — no blockers; it verified empirically that no path produces a false-deny or hang, the exemption exactly matches what CI's format job skips, and both sibling Bash hooks are preserved.

Nit #1 (the one worth fixing) is addressed in 030a7c19: the printed fix-snippet and docs by-hand check now cover staged + dirty, not just origin/main...HEAD — a brand-new staged .cs (exactly what fires the deny) was absent from that diff, so the copy-paste remedy would have stripped nothing and looped. Verified fixed on a fresh fixture.

Remaining nits accepted as-is (fail-open, Medium severity, CI backstops, and shared with the existing bash-guard/worktree-guard): git -C <path> not parsed for the target tree (only cd, matching the cd <wt> && git usage here), and the heredoc-body false-deny vector those sibling hooks also carry.

Review-verdict: MERGEABLE @ 030a7c19

Cold review returned **MERGEABLE-WITH-NITS @ 9e1fce1a** — no blockers; it verified empirically that no path produces a false-deny or hang, the exemption exactly matches what CI's `format` job skips, and both sibling Bash hooks are preserved. Nit #1 (the one worth fixing) is addressed in `030a7c19`: the printed fix-snippet and docs by-hand check now cover **staged + dirty**, not just `origin/main...HEAD` — a brand-new staged `.cs` (exactly what fires the deny) was absent from that diff, so the copy-paste remedy would have stripped nothing and looped. Verified fixed on a fresh fixture. Remaining nits accepted as-is (fail-open, Medium severity, CI backstops, and shared with the existing `bash-guard`/`worktree-guard`): `git -C <path>` not parsed for the target tree (only `cd`, matching the `cd <wt> && git` usage here), and the heredoc-body false-deny vector those sibling hooks also carry. Review-verdict: MERGEABLE @ 030a7c19

timothy merged commit 2c7330f273 into main 2026-07-17 17:50:02 +02:00

timothy deleted branch chore/bom-guard-hook 2026-07-17 17:50:02 +02:00

timothy referenced this issue from a commit 2026-07-17 17:50:03 +02:00

Merge pull request 'chore: PreToolUse guard that blocks a commit/push with a BOM'd touched .cs' (#413) from chore/bom-guard-hook into main

timothy referenced this pull request 2026-07-17 17:54:10 +02:00

feat(70): weighted / fair-share content distribution (WeightedShuffle playback order) #402

timothy referenced this pull request 2026-07-17 18:34:25 +02:00

feat(70): weighted / fair-share content distribution (WeightedShuffle playback order) #402

timothy referenced this pull request 2026-07-17 19:04:01 +02:00

Weighted / fair-share content distribution (playback order) #70

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

ad-hoc

One-off / ad-hoc work not tracked by a dedicated issue

api

REST API / HTTP endpoints

bug

Something isn't working

ci-cd

Build, test, deploy pipeline

content

Channel content / schedules / playlists

dependencies

Dependency updates (Renovate)

enhancement

New feature or improvement

frontend

ChicoryTV React SPA frontend

in-progress

Claimed by an active session — do not pick up

jellyfin

Jellyfin tuner / IPTV integration

parked

Excluded from automatic queue pickup; work only when explicitly selected

priority: high

priority: low

priority: medium

review

Adversarial review finding

security

Security / vulnerability fix

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

renovate timothy (Timothy)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: timothy/ersatztv#413