THROWAWAY — DO NOT MERGE: verify api-docs + format full paths in the CI image (#390) #400

Closed

timothy wants to merge 1 commits from tmp/390-verify-fullpaths into main

pull from: tmp/390-verify-fullpaths

merge into: :main

:main

:renovate/meziantou.analyzer-3.x

:release/v26.15.0-notes

:fix/830-add-items-error-surface

:renovate/lucene.net

:renovate/cliwrap-3.x

:issue-806-guard-populations

:renovate/dotnet-monorepo

:scratch/767b-poisoned

:scratch/767b-control

:release/v26.14.0-notes

:release/v26.14.0

:renovate/sqlitepclraw.bundle_e_sqlite3-3.x

:docs/510-skill-logo-bug-policy

:fix/510-watermark-resolution-policy

:fix/629-verdict-classifier-falseopens

:fix/609-decisions-edit-token-scope

:issue-135-clear-to-none

:release/v26.12.0-notes

:fix/409b-lastscan-api-parity

:fix/401-updatechannel-mirror-422

:fix/327-playlist-rename-validation

:fix/410-scancancel-log-level

:fix/409-447-librariesscreen-neverscanned

:fix/338-zap-exit-code

:fix/367-plex-budget-message

:fix/310-debom-legacy-cs

:ci/604-lane-rebalance

:feat/388-design-mirror

:feat/247-test-ownership

:feat/247-primary-action

:feat/357-player-owned-playback

:feat/357-jellyfin-plugin-poc

:fix/289-mcp-hardening

:issue58-mcp

:feat/244-channels-extract

:ci/auto-bump-prod-compose

:feat/multi-rerun-collections-api

:feat/collections-api

:feat/quick-wins

:feat/185-docs-part2

:feat/140-collections-screen

:feat/146-channel-edit

:feat/147-classic-ui-link

:issue22-renovate-dashboard

:feat/91-cutover

:feat/63-composite-create

:feat/65-library-browse

:feat/85-epg

:feat/86-schedule-editor

:feat/109-dashboard-data

:feat/99-session-tracking

:fix/dockerfile-node-tag

:feat/59-spa-foundation

:docs/59-ui-redesign-brief

:feat/102-json-guide

:feat/111-schedule-durations

:feat/104-artwork-upload

:feat/103-media-sources-api

:feat/playouts-read-api

:feat/108-health-api

:feat/105-picker-list-endpoints

:issue-97-channel-state-api

:issue42-jellyfin-musicvideos

:issue46-rest-api-error-contract

:dependabot/nuget/ErsatzTV.FFmpeg.Tests/multi-d307a2e06f

:qsv-improvements

:hdr-vulkan-cuda-test

Conversation 0 Commits 1 Files Changed 1

+467 -55

timothy commented 2026-07-17 12:31:59 +02:00

Owner

Copy Link

Copy Source

Throwaway PR — do not merge. Will be closed and the branch deleted once it has reported.

Exists purely to close a coverage gap the cold adversarial review raised on #399: api-docs and format passed there only via their trivial skip paths, because #399 touches no .cs and no API path. So dotnet format ErsatzTV.sln --include and ./scripts/update-openapi.sh (which needs dotnet build + python3) have never actually executed inside the new CI toolchain image.

That matters: the Blocker on #399 was a shell regression that only bit steps containing bash-isms, so "it passed" on a skip path proves nothing about the real work.

This branch is #399's head plus a comment appended to ErsatzTV/Controllers/Api/ArtistsController.cs — enough to set both api_changed=true and cs_changed=true and drive both jobs down their full paths.

Expected: both green. format proves the SDK + .editorconfig verify works in-image (the file is already de-BOM'd, so the fix-as-you-touch rule is satisfied); api-docs proves update-openapi.sh regenerates with no drift from a comment-only change.

**Throwaway PR — do not merge. Will be closed and the branch deleted once it has reported.** Exists purely to close a coverage gap the cold adversarial review raised on #399: `api-docs` and `format` passed there **only via their trivial skip paths**, because #399 touches no `.cs` and no API path. So `dotnet format ErsatzTV.sln --include` and `./scripts/update-openapi.sh` (which needs `dotnet build` + `python3`) have never actually executed **inside** the new CI toolchain image. That matters: the Blocker on #399 was a shell regression that only bit steps containing bash-isms, so "it passed" on a skip path proves nothing about the real work. This branch is #399's head plus a comment appended to `ErsatzTV/Controllers/Api/ArtistsController.cs` — enough to set both `api_changed=true` and `cs_changed=true` and drive both jobs down their full paths. Expected: both green. `format` proves the SDK + `.editorconfig` verify works in-image (the file is already de-BOM'd, so the fix-as-you-touch rule is satisfied); `api-docs` proves `update-openapi.sh` regenerates with no drift from a comment-only change.

timothy added 7 commits 2026-07-17 12:32:00 +02:00

ci(390): add shared CI toolchain image (.NET 10 SDK + Node 22 + prod ffmpeg) ... f27ff876d0

Layers the .NET 10 SDK + Node 22 onto our own ersatztv-ffmpeg base (the same
COPY --from=.../usr/share/dotnet pattern docker/Dockerfile uses for prod), and
bakes the dotnet-ef + reportgenerator global tools the jobs installed per run.

Builds/pushes 192.168.1.95:3000/timothy/ersatztv-ci:<sha> via a separate
workflow so a ~2GB toolchain image isn't rebuilt on every commit.

Refs #390

ci(390): fix CI-image smoke test — reportgenerator --version exits 1 ... 6a9e27c6b8

The build-time smoke test failed the image build even though the toolchain was
healthy: reportgenerator prints its version banner then exits 1 ("No report
files specified"), so it has no clean version probe. Probe the dotnet-tool shim
on PATH instead. dotnet --info confirmed the SDK + .NET 10.0.10 runtimes resolve
correctly on the ffmpeg base (ICU/glibc fine).

Refs #390

ci(390): run toolchain jobs in the shared CI image; drop 110s apt-ffmpeg; rebalance lanes ... afa42b0b6f

Converts test/migrations/functional-e2e/api-docs/format to run inside
ersatztv-ci:6a9e27c via container:, removing per-run setup-dotnet, setup-node,
apt-get ffmpeg and dotnet tool install. NuGet/npm caches stay (project deps);
setup-node's implicit npm cache is replaced with an explicit actions/cache.

Rolled-up wins found while auditing every job's steps (evidence in #390):
- Drop the apt-ffmpeg step (110s/run). It was never load-bearing: the app's only
  startup ffmpeg touch is a LogWarning from FFmpegLocatorService, and the harness
  drives curl-only contracts that never transcode. The image ships prod-identical
  ffmpeg 8.1.2 anyway, which #299's seeded-media follow-ups will need.
- Bake dotnet-ef + reportgenerator into the image (2 per-run tool installs gone;
  versions now live in one tracked place).
- Move api-docs (5s) and format (37s) to the idle 'small' lane — they were each
  queueing ~29min in the saturated ubuntu-latest lane. Also drops that lane from
  5 jobs to 3. Only possible because container: makes them self-contained.

Measured, not assumed: the image itself is worth only ~15-40s/job. The dominant
costs are queue wait (server-management#604) and 742s of redundant compilation
(#398) — both filed rather than silently bundled here.

Refs #390

ci(390): default run steps to bash — act_runner uses sh inside container: ... 84756eebab

The migrations job died in 0.13s with 'set: Illegal option -o pipefail'. Root
cause: inside a container: act_runner does not default to bash, it falls back to
'sh -e {0}' (dash), since it cannot assume bash exists in an arbitrary image.
Every multi-line script in this workflow is bash (set -o pipefail, arrays,
shopt, mapfile), so dash fails them immediately.

The format job passed only because it already declared shell: bash explicitly.
Declare it once at the workflow level instead of per step; non-container jobs
already defaulted to bash, so nothing changes for them.

Documented in docs/ci-cd.md as a container gotcha — it surfaces only once a job
is containerized and does not look like a shell problem.

Refs #390

ci(390): address cold review — pin-drift guard, honest cron, doc fixes ... 07048b8c96

Cold adversarial review findings:
- MEDIUM (Renovate generates pin drift): add a blocking ci-image-pin job. Renovate
  manages docker/ci/Dockerfile's base pins but cannot bump an opaque :<sha> in
  container.image, so a base bump would publish a new image, test the OLD one, and
  merge with the Dockerfile disagreeing with the pin. The guard fails when the pin
  isn't the last commit to touch docker/ci, or when the 5 jobs pin different tags —
  making the documented two-step enforced rather than remembered.
- MEDIUM (cron was a no-op): the weekly rebuild updated nothing (jobs pin :<sha>) and
  buildcache would have restored the apt layer verbatim, collecting none of the base
  updates it existed for. Added no-cache on the schedule path and rewrote the comment
  to state what it actually is: a build canary + a fresh :latest for the next bump.
- LOW: FFMPEG_TAG was referenced in the Dockerfile + docs but never existed (the FROM
  is hardcoded); reworded both.
- LOW: paths: filtered the exact file while the docs claimed docker/ci/** — use **.
- NIT: docs oversold ENTRYPOINT reset as a gotcha; act overrides it anyway. Marked
  defensive.

Refs #390

ci(390): bump pin to ersatztv-ci:07048b8; compare full shas in pin guard ... ed52892d68

Pin bump for the image rebuilt from the review-fix commit (the two-step working
as designed — the new ci-image-pin guard demanded it).

Also fixes a real bug in that guard, caught locally before CI: it compared
abbreviated shas, but git auto-scales abbreviation length with object count, so
the tag ci-image.yml builds from a fetch-depth:1 shallow clone is 7 chars while
%h on the guard's full clone is 8 — it would have failed on every run. Now
resolves both sides to full shas via rev-parse, which also catches a pin that
names no commit at all.

Verified non-vacuous locally: PASS on the real pin, FAIL on a stale pin, FAIL on
an unresolvable one.

Refs #390

tmp(390): touch an API-surface .cs to exercise api-docs + format full paths ... 639b0d40cd

THROWAWAY — do not merge. This PR exists only to prove that dotnet format
--include and scripts/update-openapi.sh actually run inside the CI toolchain
image; both jobs have so far only taken their trivial skip paths (#399 touches
no .cs and no API path), which the cold review flagged as a coverage gap.

timothy closed this pull request 2026-07-17 12:37:47 +02:00

timothy referenced this pull request 2026-07-17 12:38:35 +02:00

ci(390): shared CI toolchain image + drop 110s apt-ffmpeg + rebalance runner lanes #399

timothy referenced this pull request 2026-07-17 12:39:08 +02:00

ci(390): shared CI toolchain image + drop 110s apt-ffmpeg + rebalance runner lanes #399

timothy referenced this pull request 2026-07-17 12:41:19 +02:00

CI: shared toolchain base image (.NET 10 SDK + Node 22 + ffmpeg) for CI jobs #390

Pull request closed

This pull request cannot be reopened because the branch was deleted.

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#400