From c485f0221153aac0b5bb3ab096ad4b81b71c128d Mon Sep 17 00:00:00 2001 From: topkoa Date: Mon, 13 Jul 2026 01:07:12 -0400 Subject: [PATCH] =?UTF-8?q?ci:=20track=20the=20spec's=20HEAD=20=E2=80=94?= =?UTF-8?q?=20the=20app=20conforms=20to=20the=20living=20spec?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Design change, at the maintainer's direction: the gate now checks out feedpak-spec at HEAD instead of a pinned SHA. .feedpak-spec-ref, its 40-char validation step, and the pin-bump machinery are gone. Rationale: it is vital that the app conforms to the spec — the current spec, not a snapshot. The pin bought determinism at the cost of a maintenance loop (bump PRs, a PAT, weekly latency) and a window where the gate verified against a stale spec. Tracking HEAD makes the dev flow fully self-serve with zero upkeep: gated PR -> FEP -> spec merge -> re-run checks -> green. Nothing to bump. The trade-off is accepted with eyes open, and the docs state it: the normal FEP is additive and can only loosen the gate, so it cannot redden anyone's PR. Only a breaking spec change (rare, deliberate, MAJOR per the spec's compatibility policy) turns PRs red repo-wide — which is the correct org-wide signal that the app is out of conformance. The CI job logs the spec SHA each run verified against, so any red run is reproducible. Failure messages now also say why it matters beyond the one PR (also at the maintainer's direction): non-conformance that lands shows up as red CI on every teammate's PR until it is resolved, and only its author can clear it — the FEP route keeps everyone else unblocked. Tone softened throughout (the exceptions-file header now explains rather than shouts). Signed-off-by: topkoa --- .feedpak-spec-ref | 9 - .github/pull_request_template.md | 34 +- .github/workflows/ci.yml | 38 +- CHANGELOG.md | 7 +- CLAUDE.md | 1405 +++++++++++++++--------------- docs/feedpak-spec-gate.md | 32 +- feedpak-spec-exceptions.yml | 22 +- tools/check_spec_conformance.py | 14 +- 8 files changed, 781 insertions(+), 780 deletions(-) delete mode 100644 .feedpak-spec-ref diff --git a/.feedpak-spec-ref b/.feedpak-spec-ref deleted file mode 100644 index 4ee7faf..0000000 --- a/.feedpak-spec-ref +++ /dev/null @@ -1,9 +0,0 @@ -# Pinned commit of github.com/got-feedback/feedpak-spec that this repo is -# verified against by the `feedpak-spec` CI job (tools/check_spec_conformance.py). -# -# Pinned by SHA rather than tracking the spec's default branch on purpose: a -# change over there must never turn CI red on an unrelated PR here. When the -# spec moves, bump this SHA in its own PR — if that PR is red, the spec changed -# in a way core does not satisfy, which is exactly the signal we want, delivered -# as a reviewable PR instead of a surprise. -15e13e02062842d6660897623b41e04fd99ece58 diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 1ceb6ac..2cbac7e 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#___` — and `.feedpak-spec-ref` is bumped to the merged SHA in this PR - -## 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 7c1ab42..0d722ca 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -133,8 +133,16 @@ jobs: # 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. Pinned by SHA in .feedpak-spec-ref so a change over - # there can't redden an unrelated PR here. + # 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: @@ -152,31 +160,19 @@ jobs: python-version: '3.12' cache: 'pip' - - name: Read the pinned spec commit - id: spec - run: | - sha=$(grep -vE '^[[:space:]]*(#|$)' .feedpak-spec-ref | head -n1 | tr -d '[:space:]') - if [ -z "$sha" ]; then - echo "::error file=.feedpak-spec-ref::no commit SHA found in .feedpak-spec-ref" - exit 1 - fi - # actions/checkout resolves branches and tags in `ref` too, so a - # non-SHA here would silently un-pin the spec — the one thing this - # file exists to prevent. Demand a full 40-char SHA. - if ! printf '%s' "$sha" | grep -qE '^[0-9a-fA-F]{40}$'; then - echo "::error file=.feedpak-spec-ref::expected a full 40-character commit SHA, got '$sha' — a branch or tag name would defeat the pin" - exit 1 - fi - echo "sha=$sha" >> "$GITHUB_OUTPUT" - - - name: Check out feedpak-spec at the pinned commit + - name: Check out feedpak-spec at HEAD uses: actions/checkout@v4 with: repository: got-feedback/feedpak-spec - ref: ${{ steps.spec.outputs.sha }} + 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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 636a428..e863a8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,11 +20,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 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 spec is pinned by SHA in `.feedpak-spec-ref` so a change over there can't redden an unrelated PR - here; bump it in its own PR, and a red result is the signal that core doesn't satisfy the new spec. + 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 - bump `.feedpak-spec-ref` to the merged SHA in the same PR. `feedpak-spec-exceptions.yml` is a **closed + 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 diff --git a/CLAUDE.md b/CLAUDE.md index c8486e3..b6332ef 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,702 +1,703 @@ -# FeedBack — AI Agent Guide - -FeedBack is a self-hosted web app for browsing, playing, and practicing interactive music notation, built around its own open `.sloppak` chart format. Charts come from importing Guitar Pro (GP5/GP8) or MusicXML, or from authoring in the built-in editor. It runs as a Docker container with a FastAPI backend (`server.py`), vanilla JavaScript frontend (`static/`), shared Python libraries (`lib/`), and an extensive plugin system (`plugins/`). There are no frontend frameworks — everything is plain JS, HTML, and Tailwind CSS. - -## Architecture Quick Reference - -``` -server.py FastAPI app — library API, WebSocket highway, plugin loading -static/ - app.js Main frontend — screens, library views, player, plugin loader - highway.js Canvas note highway renderer (createHighway factory) - index.html Single-page app shell - style.css Custom CSS loaded alongside Tailwind -lib/ - song.py Core data models (Note, Chord, Arrangement, Song) - sloppak.py Sloppak format support - loosefolder.py Loose-folder XML chart support - audio.py OGG/MP3 audio handling - retune.py Pitch-shifting logic - tunings.py Tuning name/offset utilities - gp2rs.py Guitar Pro to arrangement XML conversion - gp2midi.py Guitar Pro to MIDI -plugins/ - __init__.py Plugin discovery, loading, requirements install - / Each plugin is its own directory (often a git submodule) -tests/ - test_*.py pytest test suite -``` - -## Plugin System - -Plugins are the primary extension point. Each plugin lives in `plugins//` with a `plugin.json` manifest. Plugins are typically their own git repositories — see [CONTRIBUTING.md](CONTRIBUTING.md) for the licensing policy (curated plugins should be AGPL-3.0 or AGPL-compatible: MIT, BSD, Apache-2.0). - -```json -{ - "id": "my_plugin", - "name": "My Plugin", - "version": "1.0.0", - "private": false, - "type": "visualization", - "nav": { "label": "My Plugin", "screen": "plugin-my_plugin" }, - "screen": "screen.html", - "script": "screen.js", - "styles": "assets/plugin.css", - "routes": "routes.py", - "settings": { - "html": "settings.html", - "server_files": ["my_plugin.db", "my_plugin_models/"] - }, - "diagnostics": { - "server_files": ["my_plugin.diag.json"], - "callable": "diagnostics:collect" - } -} -``` - -All fields except `id` and `name` are optional. Plugins can have any combination of frontend (screen/script), backend (routes), and settings. - -`version` and `private` are advisory metadata — the plugin loader does not currently consume them, but plugins commonly include them for publishing/tooling purposes. - -`description`, `category`, and `icon` are **optional, additive v3 Pedalboard metadata** (surfaced in `/api/plugins`, consumed by the v3 Plugins page `static/v3/plugins-page.js`). `description` is a short one-sentence summary shown under the pedal name. `category` (`audio | creation | practice | game | tools`, free-form; unknown/absent → curated default → `"other"`) picks which pedalboard the plugin sits on. `icon` is an assets-relative thumbnail path (e.g. `"assets/thumb.png"`, ~square ~256×256, same containment rule as `styles`, served via `/api/plugins//assets/...`); if omitted the loader auto-detects `assets/thumb.png`, and plugins with no thumbnail get a default pedal graphic. All three are backward-compatible — omit them and the plugin still loads. See [docs/plugin-v3-ui.md](docs/plugin-v3-ui.md). - -`styles` is the **opt-in** for self-hosted CSS (Principle II — prebuilt Tailwind, no Play CDN). Core's `static/tailwind.min.css` only contains classes scanned from core source at build time, so a plugin installed at runtime (community / NAS) that uses classes core didn't scan — especially arbitrary values like `text-[11px]` — renders unstyled. Declaring `styles` makes the frontend inject one versioned `` into `` (covering the plugin's screen *and* its settings panel) pointing at the plugin's own compiled stylesheet. The value is a **plugin-root-relative path that must live under `assets/`** (e.g. `"assets/plugin.css"`) so it serves through the sandboxed `/api/plugins//assets/...` route. Build it with `corePlugins: { preflight: false }` (utilities only — core ships the single base reset; don't duplicate it) and **never** the Tailwind Play CDN. Plugins that use only core-guaranteed utilities, or ship no Tailwind, omit `styles` and are byte-for-byte unaffected. Full authoring guide + scaffold: [docs/plugin-styles.md](docs/plugin-styles.md). - -`settings.server_files` is the **opt-in** for the unified Settings export/import flow (feedBack#113). It's a list of relpaths under `context["config_dir"]` that the plugin wants included in user-triggered backups. A trailing `/` denotes a directory (recurse). Plugins that omit this field have no server-side files exported; their state lives entirely in browser `localStorage`, which is bundled wholesale on every export. Rules: -- Relpaths only. Absolute paths, drive letters, `..` segments, and backslashes are rejected at load time with a `[Plugin]` warning. -- The same allowlist is consulted at both export and import: a bundle that references a file the *importing host*'s manifest no longer declares is skipped with a warning (handles plugin updates between export and import). A bundle that references a file your host's manifest never declared is also skipped — no surprise writes. -- Files are encoded as `{"encoding": "json", "data": }` for `.json` files that parse cleanly (diff-friendly), `{"encoding": "base64", "data": "..."}` otherwise (sqlite, model blobs, IRs). -- Plugins own their internal data migration. Importing a bundle whose data schema predates your current code restores bytes verbatim — your plugin must cope at next load. -- Symlinks are skipped on export and never followed on import. - -`diagnostics` is the **opt-in** for the troubleshooting bundle (feedBack#166 — Settings → Export Diagnostics). Two independent fields: -- `diagnostics.server_files` — same allowlist semantics as `settings.server_files`: relpaths under `context["config_dir"]`, no `..`, no abs paths, no backslashes, no leading dots. Files listed here are copied verbatim into `plugins//` inside the bundle. Use this for snapshot-style state (small DB excerpts, model lists, last-error files). -- `diagnostics.callable` — `":"` (e.g. `"diagnostics:collect"`). Resolved lazily via `load_sibling` when the user clicks Export, then called as `func({"plugin_id": "...", "config_dir": Path(...)})`. Return `dict`/`list` → written to `plugins//callable.json`; `bytes` → `callable.bin`; `str` → `callable.txt`. Exceptions are caught and appended to the bundle's `manifest.notes` — a buggy plugin never crashes the export. - -Plugins that omit the field contribute nothing to the bundle from the backend side. Frontend plugins can independently push state via `window.feedBack.diagnostics.contribute(plugin_id, payload)` from their `screen.js` before the user hits Export. Bundle layout + per-file schemas: [docs/diagnostics-bundle-spec.md](docs/diagnostics-bundle-spec.md). - -Best practices: -- Embed your own `schema` field (e.g. `"my_plugin.diag.v1"`) in JSON returned by `callable` so future tooling can dispatch by version. -- Keep payloads small (< 100 KB). Diagnostics are not a backup channel — that's `settings.server_files`. -- Don't include user secrets, API keys, or session tokens. The bundle is shared with maintainers / posted to GitHub issues. - -`type` is an optional role hint (feedBack#36). Supported values: -- `"visualization"` — plugin provides a highway renderer. Declaring this makes the plugin eligible for the main-player viz picker AND splitscreen's per-panel picker. Must pair with a `window.feedBackViz_` factory exporting the setRenderer contract below. -- Absent → no declared role; plugin is loaded and its script runs, but it doesn't appear in role-specific UIs. - -**Backend routes** — `routes.py` must export a `setup(app, context)` function. The `context` dict provides: -- `config_dir` — persistent config path -- `get_dlc_dir()` — returns the DLC folder Path -- `extract_meta()` — metadata extraction callable -- `meta_db` — shared MetadataDB instance -- `library_providers` — shared library provider registry for source-aware browsing -- `register_library_provider(provider)` — register a plugin-provided library source. Providers expose `id`, `label`, optional `kind`/`capabilities`, and callable `query_page`, `query_artists`, `query_stats`, and `tuning_names` methods. Providers with `art.read` may also expose `get_art(song_id)` returning one of: a `Response` object (any media type, served as-is); raw `bytes` or `bytearray` (**assumed PNG** — use a `Response` or a `dict` with `content`+`media_type` keys for JPEG/WebP or other formats); a URL string (http/https → 302 redirect; other schemes are rejected with 400); a filesystem path string or `Path` (served as a file with auto-detected media type); or a `dict` with a `url`, `path`, or `content` key. Providers with `song.sync` may expose `sync_song(song_id)` returning `None` (success with no local file) or a `dict` — the dict is passed through as the JSON response and should include `filename`/`local_filename` if a local playable file was produced. -- `unregister_library_provider(provider_id)` — remove a plugin-provided library source by id. The built-in `local` provider cannot be removed. -- `get_sloppak_cache_dir()` — sloppak cache path -- `load_sibling(name)` — loads a sibling module from this plugin's directory under a unique, namespaced module name. See "Sibling imports" below. -- `log` — stdlib `logging.Logger` namespaced to `feedBack.plugin.`. Pre-configured with the app-wide level, format (including JSON mode), and correlation IDs. Use this for all backend plugin output instead of `print()`. See "Backend plugin logging" below. - -**Sibling imports — use `load_sibling`, not bare imports** (feedBack#33). The plugin loader inserts each plugin's directory onto `sys.path` so `from extractor import X` works, but Python caches imports by **module name** in `sys.modules`. Two plugins that each ship a top-level `extractor.py` (or any other generic name — `util.py`, `client.py`, `parser.py`, `config.py`, …) collide: whichever loads first wins, and the other plugin's `from extractor import X` either gets the wrong module or fails with `cannot import name 'X' from 'extractor'`. - -The fix is `context["load_sibling"](name)`, which loads the sibling under a namespaced module name (`plugin_.`, where plugin_id is bijectively encoded so reverse-DNS-style ids like `com.example.foo` work without colliding: `_` -> `_5f_`, `.` -> `_2e_`) so each plugin gets its own copy: - -```python -def setup(app, context): - helper = context["load_sibling"]("helper") - HelperClass = helper.HelperClass - # … -``` - -Notes: -- `name` is a bare module name — no `.py` suffix, no slashes, no `.`. The helper raises `ValueError` for path traversal / format issues and `ImportError` for missing files. -- Both single-file siblings (`extractor.py`) and package-form siblings (`extractor/__init__.py`) work. Package form wins when both exist (matches CPython's import-resolution precedence). -- Relative imports between siblings work — `from .shared import X` in a top-level helper, `from ..shared import X` from inside a sibling package. The synthetic parent package `plugin_` carries the plugin directory in its `__path__`. -- `from . import sibling` (attribute-style) also resolves: loaded children are exposed as attributes on the parent package. -- Repeat calls return the cached module. Concurrent first-time calls are serialized via per-module locks so no caller observes a half-initialized module. -- Bare `import sibling` from `routes.py` still works during the transition period, but the loader prints a startup warning when it detects two plugins shipping a same-named top-level module — covering both `.py` files and package directories. Migrate to `load_sibling` to silence the warning and immunize your plugin from future ecosystem collisions. (Don't mix bare imports and `load_sibling` for the same module — they'd execute the file twice and split module-level state.) - -**Frontend scripts** — `screen.js` runs in the global scope via a `