From ac5c5ad20d79ec6cdf559adb294268dc66f134ad Mon Sep 17 00:00:00 2001 From: byrongamatos Date: Mon, 13 Jul 2026 13:25:44 +0200 Subject: [PATCH] ci: cover gap-fill manifest key scans Signed-off-by: byrongamatos --- .github/pull_request_template.md | 34 +- .github/workflows/ci.yml | 476 +++++----- CHANGELOG.md | 710 +++++++-------- CLAUDE.md | 1406 +++++++++++++++--------------- docs/feedpak-spec-gate.md | 265 +++--- feedpak-spec-exceptions.yml | 100 +-- tests/test_spec_gate.py | 12 + tools/check_spec_conformance.py | 1058 +++++++++++----------- 8 files changed, 2044 insertions(+), 2017 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 2cbac7e..b31237d 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,17 +1,17 @@ -## What - - - -## feedpak surface - - - -- [ ] This PR does **not** change how the app reads/writes feedpaks (manifest keys, pack files, folder layout) -- [ ] …or it does, and the spec change landed first via the [FEP process](https://github.com/got-feedback/feedpak-spec/blob/main/CONTRIBUTING.md) — FEP / spec PR: `got-feedback/feedpak-spec#___` (once it merges, re-run this PR's checks and the gate goes green) - -## Checklist - -- [ ] `CHANGELOG.md` `[Unreleased]` updated (user-visible changes) -- [ ] Tests added/updated for new behaviour -- [ ] Commits are DCO signed off (`git commit -s`) +## What + + + +## feedpak surface + + + +- [ ] This PR does **not** change how the app reads/writes feedpaks (manifest keys, pack files, folder layout) +- [ ] …or it does, and the spec change landed first via the [FEP process](https://github.com/got-feedback/feedpak-spec/blob/main/CONTRIBUTING.md) — FEP / spec PR: `got-feedback/feedpak-spec#___` (once it merges, re-run this PR's checks and the gate goes green) + +## Checklist + +- [ ] `CHANGELOG.md` `[Unreleased]` updated (user-visible changes) +- [ ] Tests added/updated for new behaviour +- [ ] Commits are DCO signed off (`git commit -s`) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0d722ca..666b122 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,238 +1,238 @@ -name: ci - -# Runs only as a reusable workflow invoked by ship-ci.yml (for PRs into main -# and release/**). It deliberately has no standalone pull_request trigger: a -# direct run would publish unprefixed "" checks, but the org rulesets -# require the "ci / " names produced when ship-ci.yml calls this workflow. -on: - workflow_call: - -permissions: - contents: read - pull-requests: read - checks: read - -jobs: - - test: - name: test - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - - uses: actions/setup-python@v5 - with: - python-version: '3.12' - cache: 'pip' - - - name: Guard against print() / traceback.print_exc() in server.py, lib/, and bundled plugin routes - run: | - # git grep: tracked files only — no .pyc / __pycache__ noise from - # later pytest runs. Covers both audited patterns from #155 / #242. - # `(^|[^A-Za-z0-9_])` anchor avoids matching suffixes like `myprint(`; - # POSIX leaves `\b` undefined, so we use an explicit character class. - hits=$(git grep -nE '(^|[^A-Za-z0-9_])(print|traceback\.print_exc)[[:space:]]*\(' \ - -- server.py lib/ \ - $(git ls-files 'plugins/*/routes.py') || true) - if [ -n "$hits" ]; then - echo "$hits" - first=$(printf '%s\n' "$hits" | head -n1) - file=$(printf '%s' "$first" | cut -d: -f1) - line=$(printf '%s' "$first" | cut -d: -f2) - echo "::error file=${file},line=${line}::print() or traceback.print_exc() found in server.py, lib/, or a bundled plugin routes.py. Use the feedBack logger (lib/logging_setup.py) — see issues #155 / #242." - exit 1 - fi - - - name: Install dependencies - run: | - python -m pip install --upgrade pip - pip install -r requirements.txt -r requirements-test.txt - - - name: Run pytest - run: pytest - - - name: Run JS plugin-API tests - run: node --test tests/js/*.test.js 'tests/plugins/*/js/*.test.js' 'plugins/*/tests/*.test.js' - - tailwind-fresh: - # Guard that the committed static/tailwind.min.css is in sync with source. - # The Play CDN's runtime JIT was removed (feedBack-desktop#110); a prebuilt - # stylesheet only contains classes the scanner saw at build time, so stale - # CSS silently ships unstyled elements. Rebuild and fail on any diff. - name: tailwind-fresh - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - - uses: actions/setup-node@v4 - with: - node-version: '20' - - - name: Rebuild Tailwind CSS - run: bash scripts/build-tailwind.sh - - - name: Verify committed static/tailwind.min.css is fresh - run: | - # Hard-fail (matches the print() guard convention) — do NOT auto-commit. - if ! git diff --quiet -- static/tailwind.min.css; then - echo "::error file=static/tailwind.min.css::static/tailwind.min.css is stale. Run 'bash scripts/build-tailwind.sh' and commit the regenerated file." - git diff -- static/tailwind.min.css - exit 1 - fi - - manifest-validation: - name: manifest-validation - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - - uses: actions/setup-python@v5 - with: - python-version: '3.12' - - - name: Validate plugin manifests - run: | - python - <<'EOF' - import json, sys - from pathlib import Path - - errors = [] - manifests = sorted(Path("plugins").glob("*/plugin.json")) - - for manifest in manifests: - try: - data = json.loads(manifest.read_text()) - except json.JSONDecodeError as e: - errors.append(f"{manifest}: invalid JSON — {e}") - continue - for field in ("id", "name"): - if field not in data: - errors.append(f"{manifest}: missing required field '{field}'") - pid = data.get("id", "") - if pid and pid != pid.lower(): - errors.append(f"{manifest}: 'id' must be lowercase, got '{pid}'") - # The plugin loader treats each plugins/ as a Python module, - # so the manifest 'id' must match its directory name. - dirname = manifest.parent.name - if pid and pid != dirname: - errors.append(f"{manifest}: 'id' ({pid!r}) must match directory name ({dirname!r})") - - if errors: - for e in errors: - print(f"::error::{e}") - sys.exit(1) - print(f"Validated {len(manifests)} manifest(s) — OK") - EOF - - feedpak-spec: - # Guard that core stays faithful to the feedpak format spec, which lives in - # its own repo (got-feedback/feedpak-spec) and is the contract third-party - # packers and players build against. Four surface checks: core reads/writes - # only manifest keys the spec declares (and the scanned-module list can't - # fall behind); the exception allowlist never grows, so the FEP process is - # the only way a new key lands; core ingests the spec's example packs; packs - # committed here pass the spec's reference validator. Motivated by - # #933, where a manifest key (`original_audio`) shipped in core without ever - # reaching the spec. - # - # The gate checks against the spec repo's HEAD, deliberately: the app must - # conform to the LIVING spec, always. The dev flow is self-serve — a gated - # PR opens a FEP, the spec PR merges, re-running this job goes green; no - # pin file to bump, nothing to maintain. Accepted trade-off: a BREAKING - # spec change (rare, deliberate, MAJOR per the spec's compatibility policy) - # reddens every PR here until core conforms — which is the correct - # org-wide signal that the app is out of conformance. The normal FEP is - # additive and can never redden this job. - name: feedpak-spec - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - # This job runs repository code (tools/check_spec_conformance.py) and - # never pushes; don't leave the token in git config for it. - # fetch-depth: 0 so the base branch is available — the gate must prove - # the exception allowlist didn't grow in this PR. - with: - persist-credentials: false - fetch-depth: 0 - - - uses: actions/setup-python@v5 - with: - python-version: '3.12' - cache: 'pip' - - - name: Check out feedpak-spec at HEAD - uses: actions/checkout@v4 - with: - repository: got-feedback/feedpak-spec - ref: main - path: .feedpak-spec - persist-credentials: false - - - name: Record the spec commit this run verified against - # HEAD-tracking means CI results can differ across time on the same - # commit. Log the exact spec SHA so a red run is reproducible. - run: git -C .feedpak-spec rev-parse HEAD - - - name: Install dependencies - run: | - python -m pip install --upgrade pip - pip install -r requirements.txt - # CI-only: the spec's reference validator needs jsonschema. Not a - # runtime dependency — this gate never runs on the serve/Docker path - # (constitution Principle I). Pinned for the same reason the spec SHA - # is: an upstream release must not turn this job red on a PR that - # changed neither this repo nor the spec. - pip install 'jsonschema==4.26.0' - - - name: Fetch the base branch's exception allowlist - id: baseline - run: | - # The allowlist is closed: it grandfathers keys that predate this gate - # and may only shrink. Prove that by diffing against the base branch — - # without this, anyone could append an entry and route around the FEP - # process from inside this repo. - # - # Resolve the base rather than hardcoding `main`: ship-ci.yml also runs - # this workflow for PRs into release/** and for pushes to release/**, - # where a main baseline would diff against the wrong branch. - # PR -> the branch it merges into - # push -> the branch itself (its tip already contains the change, so - # this is a no-op; enforcement happens at PR time) - BASE="${{ github.event.pull_request.base.ref || github.ref_name }}" - echo "diffing the allowlist against origin/$BASE" - git fetch --no-tags --depth=1 origin "$BASE" - if git cat-file -e FETCH_HEAD:feedpak-spec-exceptions.yml 2>/dev/null; then - git show FETCH_HEAD:feedpak-spec-exceptions.yml > "$RUNNER_TEMP/baseline-exceptions.yml" - echo "args=--baseline-exceptions $RUNNER_TEMP/baseline-exceptions.yml" >> "$GITHUB_OUTPUT" - else - # Only true until the PR that introduces this gate lands. - echo "args=--bootstrap-allowlist" >> "$GITHUB_OUTPUT" - fi - - - name: Check feedpak spec conformance - run: python tools/check_spec_conformance.py --spec .feedpak-spec ${{ steps.baseline.outputs.args }} - - lint: - # Maintainer/CI-only size + module-hygiene gate (constitution Principle I: - # dev tooling, never on the serve/Docker path — same category as - # scripts/build-tailwind.sh). max-lines WARNS (the 1,500-line size ratchet; - # non-blocking), while import-x/no-unresolved + no-cycle HARD-ERROR on the - # ES-module graphs the refactor produces. Exemptions: docs/size-exemptions.md. - name: lint - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - # npm ci runs third-party postinstall scripts; don't leave the token in - # git config for them (this job never pushes). - with: - persist-credentials: false - - - uses: actions/setup-node@v4 - with: - node-version: '20' - - - name: Install dependencies - run: npm ci - - - name: ESLint (size norm + module hygiene) - run: npm run lint +name: ci + +# Runs only as a reusable workflow invoked by ship-ci.yml (for PRs into main +# and release/**). It deliberately has no standalone pull_request trigger: a +# direct run would publish unprefixed "" checks, but the org rulesets +# require the "ci / " names produced when ship-ci.yml calls this workflow. +on: + workflow_call: + +permissions: + contents: read + pull-requests: read + checks: read + +jobs: + + test: + name: test + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-python@v5 + with: + python-version: '3.12' + cache: 'pip' + + - name: Guard against print() / traceback.print_exc() in server.py, lib/, and bundled plugin routes + run: | + # git grep: tracked files only — no .pyc / __pycache__ noise from + # later pytest runs. Covers both audited patterns from #155 / #242. + # `(^|[^A-Za-z0-9_])` anchor avoids matching suffixes like `myprint(`; + # POSIX leaves `\b` undefined, so we use an explicit character class. + hits=$(git grep -nE '(^|[^A-Za-z0-9_])(print|traceback\.print_exc)[[:space:]]*\(' \ + -- server.py lib/ \ + $(git ls-files 'plugins/*/routes.py') || true) + if [ -n "$hits" ]; then + echo "$hits" + first=$(printf '%s\n' "$hits" | head -n1) + file=$(printf '%s' "$first" | cut -d: -f1) + line=$(printf '%s' "$first" | cut -d: -f2) + echo "::error file=${file},line=${line}::print() or traceback.print_exc() found in server.py, lib/, or a bundled plugin routes.py. Use the feedBack logger (lib/logging_setup.py) — see issues #155 / #242." + exit 1 + fi + + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install -r requirements.txt -r requirements-test.txt + + - name: Run pytest + run: pytest + + - name: Run JS plugin-API tests + run: node --test tests/js/*.test.js 'tests/plugins/*/js/*.test.js' 'plugins/*/tests/*.test.js' + + tailwind-fresh: + # Guard that the committed static/tailwind.min.css is in sync with source. + # The Play CDN's runtime JIT was removed (feedBack-desktop#110); a prebuilt + # stylesheet only contains classes the scanner saw at build time, so stale + # CSS silently ships unstyled elements. Rebuild and fail on any diff. + name: tailwind-fresh + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-node@v4 + with: + node-version: '20' + + - name: Rebuild Tailwind CSS + run: bash scripts/build-tailwind.sh + + - name: Verify committed static/tailwind.min.css is fresh + run: | + # Hard-fail (matches the print() guard convention) — do NOT auto-commit. + if ! git diff --quiet -- static/tailwind.min.css; then + echo "::error file=static/tailwind.min.css::static/tailwind.min.css is stale. Run 'bash scripts/build-tailwind.sh' and commit the regenerated file." + git diff -- static/tailwind.min.css + exit 1 + fi + + manifest-validation: + name: manifest-validation + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-python@v5 + with: + python-version: '3.12' + + - name: Validate plugin manifests + run: | + python - <<'EOF' + import json, sys + from pathlib import Path + + errors = [] + manifests = sorted(Path("plugins").glob("*/plugin.json")) + + for manifest in manifests: + try: + data = json.loads(manifest.read_text()) + except json.JSONDecodeError as e: + errors.append(f"{manifest}: invalid JSON — {e}") + continue + for field in ("id", "name"): + if field not in data: + errors.append(f"{manifest}: missing required field '{field}'") + pid = data.get("id", "") + if pid and pid != pid.lower(): + errors.append(f"{manifest}: 'id' must be lowercase, got '{pid}'") + # The plugin loader treats each plugins/ as a Python module, + # so the manifest 'id' must match its directory name. + dirname = manifest.parent.name + if pid and pid != dirname: + errors.append(f"{manifest}: 'id' ({pid!r}) must match directory name ({dirname!r})") + + if errors: + for e in errors: + print(f"::error::{e}") + sys.exit(1) + print(f"Validated {len(manifests)} manifest(s) — OK") + EOF + + feedpak-spec: + # Guard that core stays faithful to the feedpak format spec, which lives in + # its own repo (got-feedback/feedpak-spec) and is the contract third-party + # packers and players build against. Four surface checks: core reads/writes + # only manifest keys the spec declares (and the scanned-module list can't + # fall behind); the exception allowlist never grows, so the FEP process is + # the only way a new key lands; core ingests the spec's example packs; packs + # committed here pass the spec's reference validator. Motivated by + # #933, where a manifest key (`original_audio`) shipped in core without ever + # reaching the spec. + # + # The gate checks against the spec repo's HEAD, deliberately: the app must + # conform to the LIVING spec, always. The dev flow is self-serve — a gated + # PR opens a FEP, the spec PR merges, re-running this job goes green; no + # pin file to bump, nothing to maintain. Accepted trade-off: a BREAKING + # spec change (rare, deliberate, MAJOR per the spec's compatibility policy) + # reddens every PR here until core conforms — which is the correct + # org-wide signal that the app is out of conformance. The normal FEP is + # additive and can never redden this job. + name: feedpak-spec + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + # This job runs repository code (tools/check_spec_conformance.py) and + # never pushes; don't leave the token in git config for it. + # fetch-depth: 0 so the base branch is available — the gate must prove + # the exception allowlist didn't grow in this PR. + with: + persist-credentials: false + fetch-depth: 0 + + - uses: actions/setup-python@v5 + with: + python-version: '3.12' + cache: 'pip' + + - name: Check out feedpak-spec at HEAD + uses: actions/checkout@v4 + with: + repository: got-feedback/feedpak-spec + ref: main + path: .feedpak-spec + persist-credentials: false + + - name: Record the spec commit this run verified against + # HEAD-tracking means CI results can differ across time on the same + # commit. Log the exact spec SHA so a red run is reproducible. + run: git -C .feedpak-spec rev-parse HEAD + + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install -r requirements.txt + # CI-only: the spec's reference validator needs jsonschema. Not a + # runtime dependency — this gate never runs on the serve/Docker path + # (constitution Principle I). Pinned for the same reason the spec SHA + # is: an upstream release must not turn this job red on a PR that + # changed neither this repo nor the spec. + pip install 'jsonschema==4.26.0' + + - name: Fetch the base branch's exception allowlist + id: baseline + run: | + # The allowlist is closed: it grandfathers keys that predate this gate + # and may only shrink. Prove that by diffing against the base branch — + # without this, anyone could append an entry and route around the FEP + # process from inside this repo. + # + # Resolve the base rather than hardcoding `main`: ship-ci.yml also runs + # this workflow for PRs into release/** and for pushes to release/**, + # where a main baseline would diff against the wrong branch. + # PR -> the branch it merges into + # push -> the branch itself (its tip already contains the change, so + # this is a no-op; enforcement happens at PR time) + BASE="${{ github.event.pull_request.base.ref || github.ref_name }}" + echo "diffing the allowlist against origin/$BASE" + git fetch --no-tags --depth=1 origin "$BASE" + if git cat-file -e FETCH_HEAD:feedpak-spec-exceptions.yml 2>/dev/null; then + git show FETCH_HEAD:feedpak-spec-exceptions.yml > "$RUNNER_TEMP/baseline-exceptions.yml" + echo "args=--baseline-exceptions $RUNNER_TEMP/baseline-exceptions.yml" >> "$GITHUB_OUTPUT" + else + # Only true until the PR that introduces this gate lands. + echo "args=--bootstrap-allowlist" >> "$GITHUB_OUTPUT" + fi + + - name: Check feedpak spec conformance + run: python tools/check_spec_conformance.py --spec .feedpak-spec ${{ steps.baseline.outputs.args }} + + lint: + # Maintainer/CI-only size + module-hygiene gate (constitution Principle I: + # dev tooling, never on the serve/Docker path — same category as + # scripts/build-tailwind.sh). max-lines WARNS (the 1,500-line size ratchet; + # non-blocking), while import-x/no-unresolved + no-cycle HARD-ERROR on the + # ES-module graphs the refactor produces. Exemptions: docs/size-exemptions.md. + name: lint + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + # npm ci runs third-party postinstall scripts; don't leave the token in + # git config for them (this job never pushes). + with: + persist-credentials: false + + - uses: actions/setup-node@v4 + with: + node-version: '20' + + - name: Install dependencies + run: npm ci + + - name: ESLint (size norm + module hygiene) + run: npm run lint diff --git a/CHANGELOG.md b/CHANGELOG.md index e863a8c..584c506 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,355 +1,355 @@ -# Changelog - -All notable changes to this project will be documented in this file. - -The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), -and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - -## [Unreleased] - -### Added -- **CI gate: core must stay faithful to the feedpak spec (`feedpak-spec` job).** feedpak is published as - an open format with its own repo, normative spec, JSON Schemas, and reference validator — but nothing - stopped core from reading a manifest key the spec never defined, which is exactly what happened with - `original_audio` (#583 → #933). `tools/check_spec_conformance.py` now enforces four surface properties - in CI: (1) **key-coverage** — every manifest key core reads *or writes* is declared in the spec's - `manifest.schema.json`, found by walking the AST of `lib/sloppak.py`, `lib/enrichment.py`, and - `lib/songmeta.py`, `lib/gp2notation.py`, and `lib/routers/ws_highway.py` (writes are gated too — including - `setdefault()` — and reported separately: a key core writes lands in every pack we emit, so an undeclared - one seeds the ecosystem with non-spec data; a **readers-complete** guard fails the build if that module - list falls behind the codebase); (2) **allowlist-closed** — `feedpak-spec-exceptions.yml` never grows; - (3) **forward** — core's `load_song()` ingests every example pack the spec ships; - (4) **reverse** — every pack committed here passes the spec's own `tools/validate.py` (7/7 pass today). - The gate verifies against the spec repo's **HEAD** — the app must conform to the living spec, and the - flow is self-serve: a gated PR opens a FEP, the spec PR merges, re-running checks goes green. Nothing to - pin, nothing to bump. Each run logs the spec SHA it verified against so results are reproducible. - **There is no in-repo escape hatch, by design.** A blocked PR has exactly one route: land the key in the - spec via the [FEP process](https://github.com/got-feedback/feedpak-spec/blob/main/CONTRIBUTING.md), then - re-run the PR's checks — the gate verifies against the spec's HEAD, so it goes green once the key is real. `feedpak-spec-exceptions.yml` is a **closed - grandfather list** for keys that predate the gate, not a bypass: a fourth check (**allowlist-closed**) - diffs it against the base branch and fails any PR that *adds* an entry, so it may only shrink. - `original_audio` is grandfathered there against #933 so the gate lands green and starts blocking the - *next* instance immediately; the gate takes no position on how #933 resolves (the expected outcome is - removing the key, since the spec already carries the mixdown as a stem — not adopting it). Docs: - [docs/feedpak-spec-gate.md](docs/feedpak-spec-gate.md). - -### Removed -- **The classic v2 UI shell is gone — v3 is the only UI (R3a).** `static/index.html`, the - `/v2` route, and the `FEEDBACK_UI` v2/legacy opt-out are deleted; `/` and `/v3` both serve - `static/v3/index.html`, which has been the default since 0.3.0. This is the first step of - the core-frontend ES-module migration (R3a): both shells load the same `static/app.js`, so - every subsequent step of that migration would otherwise have to be made, and verified, - twice. Removing the fallback now halves that surface before any of it is touched. - Incidentally fixes a latent bug in the old `index()` route — its guard read - `if getenv_compat("FEEDBACK_UI") or getenv_compat("FEEDBACK_UI") in ("v2", "legacy")`, - whose left operand is truthy for *any* non-empty value, so `FEEDBACK_UI=v3` actually served - the **v2** shell. `static/tailwind.min.css` is regenerated (the content globs scanned the - deleted file, so v2-only utility classes are now purged). Constitution amended to 1.3.0: - Principle II's frontend file list now names `static/v3/index.html`. - **Migration notes:** if you set `FEEDBACK_UI=v2` (or `=legacy`), or bookmarked `/v2`, there - is no longer a classic shell to fall back to — unset the variable and use `/`. The env var - itself is no longer read; the `SLOPSMITH_*`→`FEEDBACK_*` compat shim is unaffected. No - chart, settings, or plugin data changes, and no plugin API changes: v3 reuses the same - engine (`app.js`, `highway.js`, `playSong`, `showScreen`, the capability registry). - -### Fixed -- **The packaged desktop app could not start (`ModuleNotFoundError: No module named - 'appstate'`).** feedback-desktop's `scripts/bundle-slopsmith.sh` copies a *hardcoded - list* of core files into the app bundle — `server.py`, `VERSION`, `lib/`, `data/`, - `static/`, `plugins/__init__.py`. The root-level `appstate.py` and `routers/` added in - R3 shipped correctly in Docker and passed every test, and were then silently dropped - from the packaged app, which died at startup. Both now live under **`lib/`** — the one - core directory the Dockerfile (`COPY lib/`), `docker-compose.yml`, and the desktop - bundler (`cp -r lib`) all copy wholesale, and that all three put on `sys.path` (on - Windows via the embeddable-Python `._pth`, where `PYTHONPATH` is ignored). This needs no - change in feedback-desktop and no new release to take effect. Placing them there is also - correct under Principle V: with the injection seam, `appstate.py` constructs nothing and - does no import-time IO, and a route module only builds an `APIRouter`. The - `Dockerfile` / `.dockerignore` / `docker-compose.yml` entries added for the root layout - are reverted. New `tests/test_packaging.py` walks `server.py`'s module-level imports and - fails if any first-party module resolves outside a directory the packagers copy, so the - next root-level module can't ship broken. - -### Added -- **Perf harness now measures 2D-highway frame time (R3c gate).** `scripts/perf-baseline.mjs` gains a `--song` mode that reports per-frame draw-cost p50/p95/p99 (draw-tagged via `highway.addDrawHook`), the metric that gates the `highway.js` split. Maintainer/CI-only; baseline recorded in `docs/perf-baseline.md`. -- **`routers/` — extracting `server.py`'s route layer, cheapest-first (R3).** Each PR moves a cohesive route group into a `fastapi.APIRouter` under `lib/routers/`, mounted with `app.include_router(...)` at its original site (FastAPI matches in registration order; the full route table stays byte-identical). Bodies are verbatim — only the decorator receiver (`@app` → `@router`) and singleton reads (`meta_db` → `appstate.meta_db`, resolved at call time) change. So far: `audio_effects` (5), `artist_aliases` (5), `loops` (3), `playlists` (12 + covers), `ws_highway` (the 902-line highway chart WebSocket), `chart` (split/unsplit/work/fileinfo — unblocked by the DLC-path substrate), `library_extras`, `wanted`, `shop`, `progression`, `profile`, `stats` (the `/api/stats/{path}` catch-all stays registered last so it can't shadow `/recent` `/best` `/top`), `version` (`/api/version`; VERSION-file lookup adjusted for the router subdir depth), `art` (the `/api/song/{f}/art*` serve/cover-search/candidates/upload/url + `/api/art/{f}/override` routes; the shared `_song_pack_art_exists`/`_art_override_paths`/`_art_safe_name` helpers stay in `server.py` for the song/delete routes and are reached through the `appstate` seam, the CAA/release transport as `enrichment.X`), and `settings` (`GET`/`POST /api/settings`, `/reset`, and the two-phase atomic export/import bundle `/api/settings/export|import`; the shared `_default_settings` builder stays in `server.py` and is reached through the `appstate` seam), and `song` (upload/delete + the metadata write-back, user-meta, overrides, gap-fill, and per-song info routes; the scan/ingest helpers stay in `server.py` and are reached through new `appstate` seams — `kick_scan`, `invalidate_song_caches`, `stat_for_cache`, and a `scan_status()` getter — the `get_song_info` catch-all mounts after the art routes so it can't shadow them), and `library` + collections (the provider list/art/sync endpoints, the library query surface, and collection CRUD → `lib/routers/library.py`; the `LibraryProviderRegistry`/`LocalLibraryProvider`/`SmartCollectionProvider` classes + shared query/collection helpers move to `lib/library_registry.py`, and the registry instance + local provider ride the `appstate` seam — server.py still constructs the singleton and exposes `register_library_provider`/`unregister_library_provider` to plugins via `plugin_context` unchanged), and the `enrichment` route handlers (`/api/enrichment/*`: status, kick/cancel, per-song state, the Match-Review queue, and AcoustID identify → `lib/routers/enrichment.py`; the engine already lives in `lib/enrichment.py` and is reached as `enrichment.X`), and `media` (the file-serving routes — song audio `/audio/{f}`, the local-audio-path resolver `/api/audio-local-path`, and raw sloppak-member serving `/api/sloppak/{f}/file/{rel}` → `lib/routers/media.py`; the cache/static path seams were already in `appstate`), and `artist` (the artist page + external-links payload `/api/artist/{name}/page|links|links/refresh` → `lib/routers/artist.py`; MB link enrichment reached as `enrichment.X`), and `diagnostics` (`/api/diagnostics/export|preview|hardware`; the plugins-root lookup adjusted for the router subdir depth, `_running_version` reached through the `appstate` seam, pure payload-cap helpers re-exported for the `server._diag_*` tests), and `tunings` (`/api/tunings`; the pure `config.json` reader moved to `lib/appconfig.py`, the tuning-provider registry read through the `appstate` seam so plugin-contributed tunings still merge). The DLC library-path resolution (`_get_dlc_dir`, pure `_resolve_dlc_path`) moved to `lib/dlc_paths.py`, reading paths through the seam; `config_dir`/`dlc_dir`/`dlc_dir_env` now ride the `appstate` seam (env-derived, so the pop-and-reimport fixtures reconfigure it for free), and the shared request-field sanitizer `_clean_str` moved to `lib/reqfields.py`. The next cut is picked by a dependency-closure scan that ranks groups by how many `monkeypatch.setattr(server, …)` targets they'd drag along. -- **`routers/` — the first extracted route module (R3).** The five audio-effects mapping - endpoints move out of `server.py` into `lib/routers/audio_effects.py` as a - `fastapi.APIRouter`, mounted with `app.include_router(...)` **at the point in the file - where they used to be defined** — FastAPI matches routes in registration order, so the - mount site preserves it. Verified: the full 143-route table (paths, methods, *and* - order) is byte-for-byte identical to `main`. Bodies are verbatim; the only edits are - the decorator receiver (`@app.get` → `@router.get`) and the singleton read - (`audio_effect_mappings` → `appstate.audio_effect_mappings`, a module attribute - resolved at call time). This proves the seam from #833 under a real consumer, including - the second slot. The `_demo_mode_guard` middleware still blocks all four moved write - routes with 403, and `Query(...)` validation still 422s — both checked against a running - server. `server.py`: **9,445 → 9,386 lines**. -- **`lib/enrichment.py` — the metadata-enrichment subsystem leaves `server.py` (R3, move-only).** MusicBrainz / Cover-Art-Archive / AcoustID transport, the match-scorer glue, and the background enrichment worker (~930 lines, 61 defs) move out as one cohesive unit. Bodies are verbatim; the only changes are seam reads — `meta_db`/`config_dir`/`sloppak_cache_dir`/`art_cache_dir` and the two shared art helpers (`song_pack_art_exists`, `art_override_paths`, which stay in `server.py` for the art/delete routes) are reached through `appstate` at call time, and the User-Agent VERSION lookup is corrected for the module's new depth. `server.py` drives the worker through the module (`import enrichment`; the routes + scan lifecycle call `enrichment.X`); tests that faked the network on `server` now patch the same names on `enrichment` (module attribute resolved at call time, so one `setattr` reaches both the routes and the worker's internal callers). Acyclic — `enrichment` imports no `server`. Route table byte-identical; full suite green. `server.py`: 6,917 → 5,988. -- **`appstate.py` — the router seam (R3).** Route modules moving out of `server.py` - need `meta_db` and friends but must not `import server`, or the import graph goes - circular the moment `server` imports them back. So `server.py` keeps *constructing* - its singletons and now **injects** them once — `appstate.configure(meta_db=…, - audio_effect_mappings=…)` — and a router reads them back as module attributes at call - time (`import appstate; appstate.meta_db.…`). This is the Python analogue of the - frontend refactor's injected `configureX({…})` seams and of the plugin - `setup(app, context)` contract: dependencies flow one way, `server → routers → - appstate`. Two properties are load-bearing and pinned by `tests/test_appstate.py`: - (1) `import appstate` constructs nothing and touches no disk, so the ~49 test fixtures - that `sys.modules.pop("server")` + re-import (to rebuild `meta_db` under a patched - `CONFIG_DIR`) keep working untouched — a singleton *owned* by `appstate` would survive - that pop and go stale; (2) reads must be late-bound (`appstate.meta_db`, never - `from appstate import meta_db`), since a `from` import freezes the binding and defeats - both a later `configure()` and `monkeypatch.setattr` — the same read-only-binding trap - as ES `import`. `configure()` rejects an unknown slot rather than silently creating a - global nothing reads, and the suite asserts `server` actually calls it (a seam whose - wiring can no-op undetected is worse than no seam). Lives at `lib/appstate.py`. - -### Changed -- **`AudioEffectsMappingDB` moved out of `server.py` into `lib/audio_effects_db.py` - (R3, move-only).** The core-owned song/tone → provider routing index follows - `MetadataDB` out of the host file, byte-identical apart from the same constructor - seam (`__init__` takes `config_dir`; `audio_effect_mappings = AudioEffectsMappingDB(CONFIG_DIR)`), - so the module does no IO at import. The singleton stays in `server.py`; no route, - no test and no `monkeypatch.setattr(server, …)` target moves. `server.py`: - **9,705 → 9,433 lines**. -- **`MetadataDB` moved out of `server.py` into `lib/metadata_db.py` (R3, move-only).** - The library metadata cache — the `MetadataDB` class (4,018 lines) plus the query - helpers it owns (keyset paging cursors, the tuning grouping key, smart-arrangement - naming, tag normalisation, the startup DB-restore swap) — now lives in its own flat - `lib/` module. `server.py` drops from **14,037 → 9,705 lines** and keeps the - `meta_db` singleton, so `server.meta_db` and `server.app` resolve exactly as before - and every route is untouched. The only non-verbatim change is the seam that lets the - class leave `server.py`: `MetadataDB.__init__` now takes `config_dir` explicitly - (`meta_db = MetadataDB(CONFIG_DIR)`) instead of reading the module-level `CONFIG_DIR`, - which also means `lib/metadata_db.py` performs no IO at import (Principle V). Logging - still goes through the `feedBack.server` logger, so existing log filters and `caplog` - assertions resolve to the same logger object. `tests/test_settings_export_library_db.py` - now imports `_apply_pending_db_restore` from `metadata_db` (the test moved with its - subject); no other test changed. Every moved block is byte-identical to its - `server.py` original. - -### Added -- **Plugins can ship an ES-module `src/` tree (module-migration rails, R0).** The host gains three things so a plugin can move off a single global-scope `screen.js` IIFE onto native ES modules with **no build step**: (1) a new sandboxed `GET /api/plugins/{id}/src/{path}` route that serves a plugin's `src/` source subtree, containment-checked by the same `safe_join` guard as `assets/` (traversal/absolute/NUL → 404); (2) the live-edit cache contract — `Cache-Control: no-cache` + a weak mtime/size `ETag` + `If-None-Match`→`304` — applied to `src/`, `screen.js`, and `assets/` (previously `screen.js` sent no cache headers and `assets/` emitted an ETag but never revalidated), so an edited module reloads on refresh while unchanged ones `304`; and (3) `scriptType`/`minHost` passthrough from `plugin.json` to `/api/plugins`, with the loader injecting a plugin that declares `"scriptType":"module"` as `