ci: make the frontend gates real, and fix the coverage script

The repo configured four frontend gates and enforced one of them. eslint
and prettier ran in no workflow and no hook; `bun run check` ran only in
build-release.yml, so a type error could sit on master until somebody cut
a tag; and `bun run test:coverage` had been dead for months.

CI (build-and-test.yml) now runs format:check, lint, check and coverage
alongside the existing boundary and doc-link tripwires.

The coverage script failure was a version mismatch, not a config problem:
@vitest/coverage-v8 resolved to 4.1.10, whose peer range pins vitest
exactly, while package.json asked for ">=1.0.0 <5.0.0" and got 4.0.16 --
every run died on a missing BaseCoverageProvider export. The loose range
is what allowed the pair to drift, so it is now ^4.1.10.

Two ratchets, same policy as MIN_THRESHOLD in traceability-check.yml:

  eslint  --max-warnings=159   (0 errors; 159 is today's backlog, only
                                ever lower it)
  vitest  thresholds           (statements 51 / branches 45 /
                                functions 46 / lines 52, measured at
                                54.6 / 48.7 / 49.6 / 55.1)

no-console is promoted from "off" to "error": the logger-facade
migration it was waiting on is finished -- 8 calls remained, 2 of them
real stragglers in the settings page, now on the facade the file already
imported. The sink itself, tests, and scripts/ are exempted; a CLI whose
stdout is the product is not a stray debug statement.

The threshold was verified to bite by raising it to 99 and watching the
run go red, not by assuming an unfailed gate works.

DR-205 moves to Done; the coverage gate is DR-215.
This commit is contained in:
2026-08-21 18:11:26 +02:00
parent ad48d89dfe
commit 8f5c9023d0
9 changed files with 122 additions and 38 deletions
+2 -1
View File
@@ -394,7 +394,7 @@ Internal architecture, components, and application logic.
| DR-138 | Loopback is exempted from Android's cleartext ban, and nothing else is. Release builds set `usesCleartextTraffic="false"`, so the webview's request to the local media server (DR-137) was rejected by network security policy before any I/O — `<video>` failed in the same millisecond as `loadstart`, with `NETWORK_NO_SOURCE` and no server-side log at all, which is why it looked identical to a missing file. A `network-security-config` resource permits cleartext for `127.0.0.1` only and keeps `base-config cleartextTrafficPermitted="false"`, so a remote server must still be HTTPS; this is deliberately not a blanket opt-in. The manifest attribute is ignored once the config is present, so the config is the single authority. `sync-android-sources.sh` also had to learn to copy `res/xml`, which it skipped — the manifest references the resource, so a missed copy fails the resource link rather than degrading quietly | Security | UR-071 | Done |
| DR-093 | Traceability coverage gate derives its requirement denominators from `requirements.md` at run time rather than hardcoded literals: `countDefinedRequirements` counts an ID only where it leads a markdown table row (ignoring the "Traces To" column and prose) and deduplicates IDs listed both in the definition tables and in the §3 traceability matrix; `computeCoverage` reports the *intersection* of traced and defined IDs so an ID traced in code but absent from `requirements.md` is surfaced as `orphaned` instead of inflating the ratio past 100%. UT/IT test identifiers are excluded as a separate taxonomy. CI and `bun run traces:coverage` share this computation and fail on both a sub-threshold and an impossible >100% result | Tooling | - | Done |
| DR-204 | A leveled logging facade for the frontend, replacing raw `console.*` calls. One module owns the log sinks, so a level (error/warn/info/debug) decides at run time what is emitted rather than every call site deciding permanently at authoring time: a release build stays quiet, a developer chasing a playback bug turns the player's debug output on without editing and rebuilding, and nothing that reaches the console is written by a `console.log` nobody can find again. Scoped loggers carry the subsystem in the message, so a filtered console is usable while a player, a download worker and a store are all talking | Tooling | - | Proposed |
| DR-205 | ESLint + Prettier run as a gate over the frontend, so lint and formatting are decided once by configuration rather than per reviewer. Formatting is not a matter of opinion at review time, and the classes of bug a linter sees (unused bindings, floating promises, accidental globals) should never reach a human reviewer at all. Wired as an npm script so the same command runs locally and in CI, matching how `check:boundary` and the traceability gate already work | Tooling | - | Proposed |
| DR-205 | ESLint + Prettier run as a gate over the frontend, so lint and formatting are decided once by configuration rather than per reviewer. Formatting is not a matter of opinion at review time, and the classes of bug a linter sees (unused bindings, floating promises, accidental globals) should never reach a human reviewer at all. Wired as an npm script so the same command runs locally and in CI, matching how `check:boundary` and the traceability gate already work | Tooling | - | Done |
| DR-206 | The Rust toolchain is pinned in-repo (`rust-toolchain.toml`) and the pin is what both a developer's machine and CI use. Without it, `cargo fmt --check` and `cargo clippy` are run by whatever version each host happens to have, so a formatting or lint result differs between a laptop and the builder image and CI fails on a diff that was clean locally — the failure mode is a red build nobody can reproduce. The builder image carries the pinned toolchain, so pinning is a *declaration*, not a CI-time install (see the no-toolchain-installs rule) | Tooling | - | Proposed |
| DR-207 | A pre-commit hook runs the "Before Committing" gates — frontend checks and tests, `cargo fmt`, clippy, the boundary tripwire and the traceability checks — so the gates are enforced at the commit rather than discovered in CI. The gates already exist and are already documented; what is missing is that nothing runs them, which makes compliance a matter of memory. The hook is the mechanism that makes the documented list actually binding | Tooling | - | Proposed |
| DR-208 | Documentation link integrity is checked mechanically (`scripts/check-doc-links.sh`): every relative markdown link in every tracked `.md` must resolve to a file that exists on disk. This is a real defect class, not hygiene — the generated traceability matrix shipped ~2,800 dead file links because it was written to `docs/` while its hrefs were repo-root-relative, and nothing noticed for months because no check existed and nobody clicks 2,800 links. The check validates *paths*, deliberately not anchors or external URLs: anchor resolution needs a markdown renderer's slug rules and network checks make the gate flaky, so both are out of scope and stated as such in the script | Tooling | - | Done |
@@ -404,6 +404,7 @@ Internal architecture, components, and application logic.
| DR-212 | Query and URL construction bind or encode their inputs. Three sites interpolated caller-supplied values directly: the offline `get_items` item-type filter built `IN ('a','b')` by string formatting, `build_get_items_endpoint` wrote `ParentId`/`IncludeItemTypes`/`SortBy`/`SortOrder` into a URL unencoded, and `player_set_volume` accepted NaN and out-of-range floats. Each is a *consistency* defect rather than a novel one — the same file already did it correctly a few lines away (parameter placeholders in `search`, `urlencoding::encode` for genres, `clamp` in every player backend). List separators stay unencoded and encoding is per element, because Jellyfin splits these parameters on the comma | Repository | UR-007, UR-065 | Done |
| DR-213 | Containerised builds hand their artifacts back to the host user. The compose services bind-mount the repo and run as root — their caches live at `/root/.cargo` and `/root/.bun`, so a non-root container user cannot write them — which leaves root-owned files accumulating in the developer's working tree: 11,124 of them when this was found, enough that `cargo clean` and `scripts/clean.sh` failed with EACCES and a plain `cargo build` died part-way, since build scripts compile for the host and land in `target/debug` even during a cross-build. Ownership is restored at the end of each containerised build, reading the intended owner from the checkout so no uid needs plumbing through. Running the containers as the host uid is the tidier fix and remains open; it needs the cache volumes relocated off `/root` first | Tooling | - | Done |
| DR-214 | The app identifies itself correctly everywhere a user or a package manager reads its name. `productName` was the scaffold's lowercase `jellytau`, which is what the Android release build showed under its icon and what the deb/rpm/NSIS bundles carried as their display name — invisible in development because `build.gradle.kts` overrides the label to "JellyTau Debug" for the debug build type, so the install a developer looks at daily was the only correctly-cased one. `mainBinaryName` pins the executable filename so nothing that resolves a path by name has to change. `strings.xml` moves into the canonical android tree, where `sync-android-sources.sh` already copies `res/values/*.xml`, so the fix survives regenerating `gen/`. Bundle metadata (publisher, copyright, category, descriptions, licence) was entirely absent, which is why the packages shipped with no maintainer or description — the hand-written Arch PKGBUILD and `.desktop` had all of it, so only the *generated* packaging was wrong | Packaging | - | Done |
| DR-215 | Frontend test coverage is a ratcheted CI gate rather than a number nobody looks at. `test:coverage` had been configured since the suite was created and was silently broken: `@vitest/coverage-v8` resolved to 4.1.10, whose peer range pins `vitest` exactly, while `package.json` asked for `>=1.0.0 <5.0.0` and got 4.0.16 — so every invocation died on a missing `BaseCoverageProvider` export and no coverage figure had been produced in months. Fixing the range is half the requirement; the other half is that a measured figure that gates nothing decays the same way an unrun script does. Thresholds sit a few points under the measured result (statements 54.6, branches 48.7, functions 49.6, lines 55.1 when this landed) and only ever move up, matching `MIN_THRESHOLD` in the traceability gate and the eslint `--max-warnings` ratchet. The absolute numbers are held down by `.svelte` components, which this project deliberately does not test directly — the pattern is to extract the logic to a plain module and test that | Tooling | - | Done |
| DR-198 | The webview runs under a real Content-Security-Policy, and the asset protocol is scoped to the one directory it still serves. `csp` was `null`, which disables CSP entirely: any script that reached the web layer — through a future `{@html}`, a dependency, or a devtools paste — would have inherited the whole IPC surface, and with it the user's session. `script-src 'self'` (Tauri injects a nonce for SvelteKit's inline bootstrap script at build time, so no `'unsafe-inline'` is needed) plus `object-src`/`frame-src 'none'` and `base-uri 'self'` is the part that is genuinely restrictive. `img-src`/`media-src`/`connect-src` cannot be: the Jellyfin origin is typed in by the user at run time and is commonly plain `http` on a LAN, so they allow `http:`/`https:` — a wide grant for *data*, but one that still bars `file:`, `filesystem:` and scripting schemes, and leaves `script-src` untouched. `style-src` keeps `'unsafe-inline'` because Svelte compiles `style="…"` attributes (including `app.html`'s `display: contents` wrapper) into markup; this is safe only while no `<style>` element survives into `index.html`, since a nonce there would make Tauri's injection outrank — and therefore void — `'unsafe-inline'`. `worker-src blob:` and `media-src blob:` are hls.js: it demuxes in a worker built from a blob and attaches MSE through `URL.createObjectURL`. `asset:` and `http://asset.localhost` are the same protocol under the two naming schemes `convertFileSrc` emits (custom scheme on Linux/macOS, `http` host on Windows/Android); `ipc:`/`http://ipc.localhost` is the invoke transport, which would otherwise be blocked by `connect-src`. A run-time CSP naming the server origin exactly was rejected: Tauri computes the header from immutable config when it serves the HTML, so it would mean rebuilding config and reloading the webview on every server change, for a policy the user can already point anywhere. The asset-protocol scope narrows from `$APPDATA/**` to `$APPDATA/thumbnails/**` — since DR-137 moved downloaded media to the loopback server, `imageCache` is the only `convertFileSrc` caller left, so the database and the encrypted-token fallback file no longer sit inside the grant | Security | UR-012, UR-071 | Done |
---